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

Simple filters change between panel and drilldown panel #3568

Merged
merged 7 commits into from
Sep 6, 2021

Conversation

CPAlejandro
Copy link
Contributor

@CPAlejandro CPAlejandro commented Aug 18, 2021

Hi guys,

In this PR we try to change the simple filters between the vies of Panel and the view of Drilldown Panel (selecting a row in Panel)

To test it, go to GitHub Panel, see the Simple filters allocated there, then select a row and check if the filters have changed. Of course, only some of these rows have to change. Do more than 1 attempt.

Example:

  • Panel view:
    image

  • Drilldown view:
    image

Closes #3563

@CPAlejandro CPAlejandro self-assigned this Aug 18, 2021
Copy link
Member

@mpRegalado mpRegalado left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only comment I think would block this PR is the type for the new prop in useSuggestedValues. The rest are either suggestions or questions for curiosity's sake.

Copy link
Member

@asteriscos asteriscos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test: ❌
Tested the PR and found this strange behavior:
After selecting different values and clicking one of the table rows with a different field of the selected ones, the combo-box does not show any selected items. The selection counter is ok, but in the items list there's none selected.

Peek 2021-08-18 15-37

Copy link
Member

@mpRegalado mpRegalado left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CR: LGTM ✔️
Test: LGTM ✔️

Copy link
Member

@asteriscos asteriscos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ✔️

@asteriscos asteriscos self-assigned this Sep 3, 2021
Copy link
Member

@asteriscos asteriscos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test ❌

Tested it on fields which have +500 values and it is not possible to select values different ranges.
Seems the last selection replaces the previous one.

Peek 2021-09-03 18-28

@frankeros
Copy link
Contributor

Test x

Tested it on fields which have +500 values and it is not possible to select values different ranges.
Seems the last selection replaces the previous one.

Peek 2021-09-03 18-28

Please create a new issue with this.

@frankeros frankeros merged commit 759c5f3 into feat/3372-github-module Sep 6, 2021
@frankeros frankeros deleted the feat/3563-simple-filters-change branch September 6, 2021 19:38
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.

None yet

4 participants