8000 [ENHANCEMENT] Table supports legend variation of selection behavior by juliepagano · Pull Request #1261 · perses/perses · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[ENHANCEMENT] Table supports legend variation of selection behavior #1261

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jun 21, 2023
Merged

[ENHANCEMENT] Table supports legend variation of selection behavior #1261

merged 5 commits into from
Jun 21, 2023

Conversation

juliepagano
Copy link
Contributor
@juliepagano juliepagano commented Jun 21, 2023 •
edited
Loading
  • Table component now includes an optional rowSelectionVariant prop that can be used to change the row selection behavior.
  • The 'standard' rowSelectionVariant (default) behaves using standard checkbox behavior (clicking a checkbox toggles selection of the row).
  • The 'legend' rowSelectionVariant follows the legend behavior of "focusing" the selected row. When a modifier key (meta or shift) is pressed, it will behave like 'standard'.
  • The 'table' mode of Legend now uses the 'legend' rowSelectionVariant.

Checklist

  • Pull request has a descriptive title and context useful to a reviewer.
  • Pull request title follows the [<catalog_entry>] <commit message> naming convention using one of the following catalog_entry values: FEATURE, ENHANCEMENT, BUGFIX, BREAKINGCHANGE, IGNORE.
  • All commits have DCO signoffs.

UI Changes

  • Changes that impact the UI include screenshots and/or screencasts of the relevant changes.
  • Code follows the UI guidelines.
  • Visual tests are stable and unlikely to be flaky. See Storybook and e2e docs for more details. Common issues include:
    • Is the data inconsistent? You need to mock API requests.
    • Does the time change? You need to use consistent time values or mock time utilities.
    • Does it have loading states? You need to wait for loading to complete.

Screenshots

Showing list and table legend selecting behaving the same

Screen.Recording.2023-06-20.at.5.23.02.PM.mov

Showing two variants of selection for table

Screen.Recording.2023-06-20.at.5.24.40.PM.mov

Showing table legend with new behavior in a time series panel

Screen.Recording.2023-06-20.at.5.26.16.PM.mov

@juliepagano
[ENHANCEMENT] Table supports legend variation of selection behavior
63974fc 
- `Table` component now includes an optional `rowSelectionVariant` prop
  that can be used to change the row selection behavior.
- The 'standard' `rowSelectionVariant` (default) behaves using standard
  checkbox behavior (clicking a checkbox toggles selection of the row).
- The 'legend' `rowSelectionVariant` follows the legend behavior of
  "focusing" the selected row. When a modifier key (meta or shift) is
  pressed, it will behave like 'standard'.
- The 'table' `mode` of `Legend` now uses the 'legend'
  `rowSelectionVariant`.

Signed-off-by: Julie Pagano <julie.pagano@chronosphere.io>
juliepagano
juliepagano commented Jun 21, 2023
View reviewed changes
ui/components/src/Legend/Legend.stories.tsx
@@ -260,7 +260,7 @@ export const SelectedItems: StoryObj<LegendProps> = {
{legendModes.map((mode) => {
return (
<StorySection key={mode} title={mode} level="h4">
<LegendWrapper
<UncontrolledLegendWrapper
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Swapping this to make it easier to see the selection working.

juliepagano
juliepagano commented Jun 21, 2023
View reviewed changes
ui/components/src/Table/Table.tsx
Comment on lines +65 to +66
// Note that this behavior needs to be kept in sync with behavior in
// the Legend component for list-based legends.
Copy link
Contributor Author
@juliepagano juliepagano Jun 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

There really isn't a great way to use a single util for this because the table and legend list have some different interaction patterns and technical implementations.

juliepagano
juliepagano commented Jun 21, 2023
View reviewed changes
ui/components/src/Table/model/table-model.ts
*
* @default 'standard'
*/
rowSelectionVariant?: 'standard' | 'legend';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a variant instead of just modifying the table behavior because the "legend" behavior is kind of weird for checkboxes and probably isn't want we want in non-legend table use cases.

@juliepagano juliepagano marked this pull request as ready for review June 21, 2023 00:29
@juliepagano juliepagano requested review from a team, cndonovan and saminzadeh June 21, 2023 00:30
@juliepagano
Merge remote-tracking branch 'origin/main' into juliepagano/table-leg…
3fba56d 
…end-selection
@juliepagano
Fix merge issues
e7afd01 
Signed-off-by: Julie Pagano <julie.pagano@chronosphere.io>
@juliepagano
Add back change lost in merge conflict
d960e01 
Signed-off-by: Julie Pagano <julie.pagano@chronosphere.io>
@juliepagano
Copy link
Contributor Author

I need to do some syncing with @foxbat07 on the behavior here. Please do not merge this until I follow-up on that late today or early tomorrow.

@juliepagano juliepagano changed the title [ENHANCEMENT] Table supports legend variation of selection behavior DONOTMERGE: [ENHANCEMENT] Table supports legend variation of selection behavior Jun 21, 2023
@juliepagano
Adjust checkbox size
caf6569 
Signed-off-by: Julie Pagano <julie.pagano@chronosphere.io>
@juliepagano juliepagano changed the title DONOTMERGE: [ENHANCEMENT] Table supports legend variation of selection behavior [ENHANCEMENT] Table supports legend variation of selection behavior Jun 21, 2023
@juliepagano
Copy link
Contributor Author

I synced with @foxbat07, and we agreed this was a good incremental improvement. He'll sync up with some other designers, and we may iterate a bit in follow-ups, but this is good for now. I also adjusted the checkbox size a little bit based on his feedback while we were reviewing.

@juliepagano juliepagano merged commit d59befd into main Jun 21, 2023
@juliepagano juliepagano deleted the juliepagano/table-legend-selection branch June 21, 2023 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjcobb sjcobb sjcobb approved these changes

@cndonovan cndonovan Awaiting requested review from cndonovan

@saminzadeh saminzadeh Awaiting requested review from saminzadeh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@juliepagano @sjcobb
0