Skip to content
This repository has been archived by the owner on Jan 19, 2023. It is now read-only.

Allow table sorting by multiple keys #1566

Merged
merged 3 commits into from
Nov 24, 2020

Conversation

GuessWhoSamFoo
Copy link
Contributor

@GuessWhoSamFoo GuessWhoSamFoo commented Oct 30, 2020

What this PR does / why we need it:
This PR adds sorting with multiple keys and sorts items with an error status by default in order to increase visibility of unhealthy cluster items. This is a breaking API change as table.Sort is in pkg

Since datagrids are paginated, this results in a poor user experience if the status of an object changes while a user is looking at it. We will need to rethink if the goal should be filtering instead.

Which issue(s) this PR fixes

Signed-off-by: GuessWhoSamFoo [email protected]

@GuessWhoSamFoo GuessWhoSamFoo self-assigned this Oct 30, 2020
@GuessWhoSamFoo GuessWhoSamFoo force-pushed the sort-status branch 2 times, most recently from 84e5c1a to 8fe97ca Compare October 30, 2020 14:21
@GuessWhoSamFoo GuessWhoSamFoo marked this pull request as draft October 30, 2020 18:37
@GuessWhoSamFoo GuessWhoSamFoo changed the title Move error and warning statuses to top of datagrid Allow table sorting by object status and multiple keys Oct 30, 2020
@GuessWhoSamFoo GuessWhoSamFoo marked this pull request as ready for review October 30, 2020 22:48
@GuessWhoSamFoo GuessWhoSamFoo force-pushed the sort-status branch 2 times, most recently from f60963a to 0f97b36 Compare November 2, 2020 18:15
@GuessWhoSamFoo GuessWhoSamFoo changed the title Allow table sorting by object status and multiple keys Allow table sorting by multiple keys Nov 2, 2020
@GuessWhoSamFoo
Copy link
Contributor Author

there was discussion about how this might look more maintainable if we separated out the reverse param into its own method and have a sort that only requires one or more keys

@GuessWhoSamFoo
Copy link
Contributor Author

Added table.Reverse() and removed the boolean from sort

@GuessWhoSamFoo GuessWhoSamFoo requested a review from a team November 12, 2020 18:46
Copy link
Contributor

@wwitzel3 wwitzel3 left a comment

Choose a reason for hiding this comment

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

Please add a change log entry for this and lets document this in the storybook stories?

GuessWhoSamFoo added 3 commits November 24, 2020 10:51
Signed-off-by: GuessWhoSamFoo <[email protected]>
Signed-off-by: GuessWhoSamFoo <[email protected]>
@GuessWhoSamFoo GuessWhoSamFoo merged commit e35deca into vmware-archive:master Nov 24, 2020
@GuessWhoSamFoo
Copy link
Contributor Author

@alexbrand Sorry this took a while to get merged!

@GuessWhoSamFoo GuessWhoSamFoo deleted the sort-status branch November 24, 2020 21:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants