Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENHANCEMENT] Query String: Improve Filter Control and add prefix (for usage w/ multiple tables) #1566

Merged

Conversation

dansysanalyst
Copy link
Member

@dansysanalyst dansysanalyst commented May 23, 2024

⚡ PowerGrid - Pull Request

Welcome and thank you for your interest in contributing to our project!. You must use this template to submit a Pull Request or it will not be accepted.


Motivation

  • Bug fix
  • Enhancement
  • New feature
  • Breaking change

Description

This Pull Request aims to improve the filter control while using Query String and adds the possibility to prefix fields, to use query string with more than one component in the same page.

Filter control

BEFORE

CleanShot 2024-05-23 at 13 07 57@2x

🚀 AFTER

CleanShot 2024-05-23 at 14 32 21@2x

Prefix Query String (Multiple Components)

CleanShot 2024-05-24 at 01 57 06@2x

When using multiple components, the user must pass the parameter $prefix to the powerGridQueryString(). As a result, each URL parameter will be prefixed with the provided prefix:

class DemoTable extends PowerGridComponent
{
    protected function queryString()
    {
        return $this->powerGridQueryString('table1');
    }
http://powergrid-demo.test/two-tables-in-the-same-page?table1_name=ark&table2_name=a&table2_calories=true

While we could use the $tableName as a prefix, I think it's best to give the user control so they can decide what is in the URL. Some people my prefer "table_name" instead of "dish_table_name".

I could not find a way to write proper tests for query-string.

Related Issue(s): #1563

Documentation

This PR requires Documentation update?

  • Yes
  • No
  • I have already submitted a Documentation pull request.

@dansysanalyst dansysanalyst marked this pull request as draft May 23, 2024 00:22
@dansysanalyst dansysanalyst changed the title POC Improve Filter Control with Query String May 23, 2024
@dansysanalyst
Copy link
Member Author

CleanShot 2024-05-23 at 11 18 24@2x

@luanfreitasdev , what are your thoughts on adding a background color to the "clear all" to differentiate it from the individual filter cleaning buttons?

@luanfreitasdev
Copy link
Collaborator

CleanShot 2024-05-23 at 11 18 24@2x

@luanfreitasdev , what are your thoughts on adding a background color to the "clear all" to differentiate it from the individual filter cleaning buttons?

Done. Thank you @dansysanalyst !

@dansysanalyst dansysanalyst changed the title Improve Filter Control with Query String Query String: Improve Filter Control and add prefix (for usage w/ multiple tables) May 24, 2024
@dansysanalyst dansysanalyst force-pushed the improve_query_string branch from f761a41 to 4ccda3d Compare May 24, 2024 09:38
@dansysanalyst dansysanalyst changed the title Query String: Improve Filter Control and add prefix (for usage w/ multiple tables) [ENHANCEMENT] Query String: Improve Filter Control and add prefix (for usage w/ multiple tables) May 24, 2024
@dansysanalyst dansysanalyst marked this pull request as ready for review May 24, 2024 09:41
@dansysanalyst dansysanalyst force-pushed the improve_query_string branch from 4ccda3d to 0d83ca7 Compare May 24, 2024 09:43
@luanfreitasdev luanfreitasdev merged commit 643da0a into Power-Components:5.x May 24, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants