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

Table: Add select/unselect all column values to table filter #79290

Merged

Conversation

ahuarte47
Copy link
Contributor

@ahuarte47 ahuarte47 commented Dec 10, 2023

What is this feature?

This MR adds two buttons to easily select or unselect subset of columns values in Table panels:

  • New Select values button:
    To select the subset of column values currently displayed.
  • New Unselect values button:
    To unselect the subset of column values currently displayed.

Why do we need this feature?

When using big tables there is often a need to filter by values to select many entries. Then you use the filter you can find e.g. 100 of values and there is no possibility to select them all with one click, you need to check them one by one, that is not feasible with such a large number of entries.

Who is this feature for?

Users filtetring large list of values in tables.

Which issue(s) does this PR fix?:

Fixes #79371

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@ahuarte47 ahuarte47 requested a review from a team December 10, 2023 10:29
@ahuarte47 ahuarte47 requested review from oscarkilhed and baldm0mma and removed request for a team December 10, 2023 10:29
@CLAassistant
Copy link

CLAassistant commented Dec 10, 2023

CLA assistant check
All committers have signed the CLA.

@grafana-pr-automation grafana-pr-automation bot added type/docs area/frontend pr/external This PR is from external contributor labels Dec 10, 2023
@imatwawana imatwawana added this to the 10.3.x milestone Dec 11, 2023
@imatwawana imatwawana added the no-backport Skip backport of PR label Dec 11, 2023
Copy link
Collaborator

@imatwawana imatwawana left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I'm just reviewing the documentation updates, so there will still need to be a technical review. I've made a couple grammar edits but mostly tried to simplify some of the language. I think we probably also need to think about changing the names of the buttons, but I'd need to see the implementation first (hopefully when all the checks pass, the system will provide a preview).

@ahuarte47
Copy link
Contributor Author

thank you very much @imatwawana for you review and help.

Copy link
Contributor

@oscarkilhed oscarkilhed left a comment

Choose a reason for hiding this comment

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

Hey, thank you for your contribution!

image

This is useful and the code looks good at first glance (I'll look closer soon). But I think we could improve the UI and the wording on these buttons. @catherineymgui, is this something to bring to one of the UX feedback sessions?

@catherineymgui
Copy link
Contributor

Yes, please bring it to a UX feedback session!

@ahuarte47
Copy link
Contributor Author

ahuarte47 commented Dec 13, 2023

Hi there, please, may I propose a new feature in this PR? I have added a new ListBox to alllow users to select what items are displayed using comparison expressions, not only using regex patterns.

It is useful when you need to filter values like numbers.

This new commit shows a list box (In order to do not alter the behavior in other panels, there is in the component a new showOperators flag disabled by default).

Screenshot from 2023-12-13 18-44-12

Screenshot from 2023-12-13 18-44-46

I still need to fix an issue with the Select component, I do not know why it closes its parent FilterPopup component when you click an item in the list of operators. Please, do you know why?

What do you think? It can be useful to include? If you disagree, I revert this change.
Thanks in advance

UPDATE:
The Contains operator is the current filter using regex, and it is the default operator.

@tjanson
Copy link

tjanson commented Dec 13, 2023

@ahuarte47 I'm merely a humble user, but I've been looking for this kind of feature for ages. It seems clearly outside the scope of this issue, but while we're at it:

  • For numeric values, it would be extremely useful to filter by ranges (score > 7, 3 < score < 7),
  • for text values a simple search would be useful (description contains "openssl"),
  • for timestamps, time ranges (date before 2022-02-02), and so on.

This can be extended further and further (why not auto-bucket numeric values? arbitrary regular expressions?), but what you propose would already be hugely beneficial.

@ahuarte47 ahuarte47 force-pushed the main_table-select-unselect-values branch from dcb11b9 to 2061652 Compare December 13, 2023 18:34
@ahuarte47
Copy link
Contributor Author

ahuarte47 commented Dec 13, 2023

@ahuarte47 I'm merely a humble user, but I've been looking for this kind of feature for ages. It seems clearly outside the scope of this issue, but while we're at it:

* For numeric values, it would be extremely useful to filter by ranges (`score > 7`, `3 < score < 7`),

* for text values a simple search would be useful (`description contains "openssl"`),

* for timestamps, time ranges (`date before 2022-02-02`), and so on.

This can be extended further and further (why not auto-bucket numeric values? arbitrary regular expressions?), but what you propose would already be hugely beneficial.

Hi @tjanson this extra filter could be implemented in new operator "expression" in this list, but I am not an expert grafana & react developer, any help would be great! I hope I haven't already made a mess in the code.

@ahuarte47 ahuarte47 force-pushed the main_table-select-unselect-values branch 2 times, most recently from 1b65af9 to fecd54a Compare December 14, 2023 07:26
@oscarkilhed
Copy link
Contributor

oscarkilhed commented Dec 14, 2023

I think we should keep this PR to the original scope. Smaller incremental improvements are generally preferred.

I brought this change to the UX feedback session and the general consensus was that a UX similar to the github inbox would work better.

image
image
image

@ahuarte47
Copy link
Contributor Author

I think we should keep this PR to the original scope. Smaller incremental improvements are generally preferred.

OK, great! I remove that extra temporal commit.

@ahuarte47 ahuarte47 force-pushed the main_table-select-unselect-values branch from fecd54a to 26f30b5 Compare December 14, 2023 13:08
@reindlt
Copy link

reindlt commented Dec 15, 2023

Hi, seems to be a similiar functionality as in #79072. If this one is accepted I think the other one could be omitted.

@ahuarte47 ahuarte47 force-pushed the main_table-select-unselect-values branch from 26f30b5 to e4fe747 Compare December 19, 2023 12:50
@ahuarte47
Copy link
Contributor Author

I brought this change to the UX feedback session and the general consensus was that a UX similar to the github inbox would work better.

Hi, I have refactorized this PR, now it is working with a three-state Checkbox:
image

@oscarkilhed
Copy link
Contributor

@ahuarte47 Thank you, I will get back to this soon!

@imatwawana
Copy link
Collaborator

imatwawana commented Dec 20, 2023

@ahuarte47 - After @oscarkilhed has completed his review and any changes are implemented, please request review from me again, so I can take another look at the docs with the updated UI. Just FYI, I'll be away until January 2nd after today, so it might be in a couple weeks. In the meantime I've added a label to keep the PR from going stale.

@imatwawana imatwawana added the no stalebot exempt from stalebot label Dec 20, 2023
@ahuarte47
Copy link
Contributor Author

ahuarte47 commented Dec 21, 2023

@tjanson

@ahuarte47 I'm merely a humble user, but I've been looking for this kind of feature for ages. It seems clearly outside the scope of this issue, but while we're at it:

* For numeric values, it would be extremely useful to filter by ranges (`score > 7`, `3 < score < 7`),

* for text values a simple search would be useful (`description contains "openssl"`),

* for timestamps, time ranges (`date before 2022-02-02`), and so on.

This can be extended further and further (why not auto-bucket numeric values? arbitrary regular expressions?), but what you propose would already be hugely beneficial.

I have created a new feature-request issue: #79800 I will try to propose a PR soon.

Copy link
Contributor

@oscarkilhed oscarkilhed left a comment

Choose a reason for hiding this comment

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

I think this is close to mergable.

I've created a pull request (ahuarte47#1) to your branch to align the select all checkbox with the rest of the checkboxes.

I also removed the "Click to" bit in the checkbox label as I found it superfluous.

Align select all checkbox and fix wording
@oscarkilhed oscarkilhed changed the title Add select/unselect column values buttons to table panel Table: Add select/unselect column values buttons to table panel Jan 2, 2024
@oscarkilhed oscarkilhed changed the title Table: Add select/unselect column values buttons to table panel Table: Add select/unselect all column values to table filter Jan 2, 2024
Copy link
Contributor

@oscarkilhed oscarkilhed left a comment

Choose a reason for hiding this comment

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

This looks good. When @imatwawana has gone over the docs we can merge this.

Copy link
Collaborator

@imatwawana imatwawana left a comment

Choose a reason for hiding this comment

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

One quibble about where the box is located. If I'm wrong, please update my suggestion accordingly and commit to add the text formatting for the buttons.

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
@ahuarte47
Copy link
Contributor Author

Thank you very much @imatwawana @oscarkilhed for your help!

@oscarkilhed oscarkilhed merged commit 40583ae into grafana:main Jan 4, 2024
17 checks passed
@ahuarte47
Copy link
Contributor Author

Thanks @oscarkilhed @imatwawana for Your help!

@imatwawana
Copy link
Collaborator

imatwawana commented Jan 4, 2024

Thanks for the contribution, @ahuarte47! I was literally wishing for the features you've added just a couple weeks ago!

@ahuarte47 ahuarte47 deleted the main_table-select-unselect-values branch January 5, 2024 17:37
zserge pushed a commit that referenced this pull request Jan 22, 2024
* Add/Remove columns values to the filter using a UX similar to the github inbox

* Align select all checkbox and fix wording

* Update docs/sources/panels-visualizations/visualizations/table/index.md

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>

---------

Co-authored-by: Oscar Kilhed <oscar.kilhed@grafana.com>
Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
@summerwollin summerwollin modified the milestones: 10.3.x, 10.3.0 Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to changelog area/frontend no stalebot exempt from stalebot no-backport Skip backport of PR pr/external This PR is from external contributor type/docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FeatureRequest] Table column filter by values "check all" functionality
8 participants