Add sorting to Table component
#2669
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jamie-suse
added
enhancement
dottorblaster
requested changes
Jun 5, 2024
There was a problem hiding this comment.
This is great 🔥
I just have one more request but we can include that in a subsequent PR: could we make adopting of sorting easier by creating a function that just gives you a sorting function out of the box? Something like the one you made for the story, but actually available to developers to import.
assets/js/common/Table/Table.jsx
Outdated
Comment on lines
144
to
165
| const displaySortIcons = ({ | ||
| sortable = false, | ||
| sortDirection = undefined, | ||
| }) => { | ||
| if (!sortable) return null; | ||
|
|
||
| if (sortDirection === 'asc') | ||
| return ( | ||
| <span className="inline-table relative top-1"> | ||
| <EOS_ARROW_UPWARD /> | ||
| </span> | ||
| ); | ||
|
|
||
| if (sortDirection === 'desc') | ||
| return ( | ||
| <span className="inline-table relative top-1"> | ||
| <EOS_ARROW_DOWNWARD /> | ||
| </span> | ||
| ); | ||
|
|
||
| const renderedData = pagination | ||
| ? page(currentPage, filteredData) | ||
| : filteredData; | ||
| return null; | ||
| }; |
There was a problem hiding this comment.
This could be a React component on its own maybe? This way you could use it with <SortIcon sortable={sortable} direction={sortDirection} />
dottorblaster
pushed a commit
that referenced
this pull request
Jun 27, 2024
dottorblaster
pushed a commit
that referenced
this pull request
Jun 28, 2024
dottorblaster
added a commit
that referenced
this pull request
Jun 28, 2024
* Add PatchList component (#2623) * Port relevant_patch_factory from Elixir to JavaScript This commit adds a ported version of the relevant_patch_factory, so it is available in tests and Storybook. * Add PatchList component and accompanying stories This commit adds the PatchList component. It will become the basis for the relevant patches page. * Implement changes requested for merge * Make eslint happy * Add jest test for PatchList * Remove Link component from PatchList This commit removes the Link component from React Router DOM and replaces it with an onNavigate function. This way the component becomes encapsulated from the responsibility of handling the navigation. * Move onNavigate from the patches array to a prop This makes the API way cleaner from a callee perspective. * Patches for a single package plus errata details infrastructure (#2643) * Add factories * Add http ground work for errata details and patches for package * Add callbacks to discovery, patches for package function to the context * Add tests * Fix missing label in tests * Change relevant_patches_hosts to relevant_patches_system_ids * LMAO logging * Add upgradable packages list component (#2636) * Fix empty body inside SUMA http executor (#2648) * Add host patches page (#2649) * Filter patches on HostRelevantPatches This commit introducses filtering by advisory and synapsis and adds a suppoprting story. * Clean up styling and rename host to hostName * Add HostRelevantPatches page and state selectors * Update routes to include HostRelevantPatches * Resolve and add TODOs with Alessio This commit adds TODOs and resolves open questions * Fix formatting * Fix Storybook story after renaming a variable Furthermore, this patch removes a wrapper container for the story, as that is nod needed anymore * Add relevant patches factory to common export * Fix linting errors * Remove debugging styles * Add Jest test for HostRelevantPatches page * Add test for the selector and rename HostRelevantPatches * Add tiny fixes to host relevant patches page (#2653) * Add patches for packages to SUSE Manager controller (#2654) * Add upgradable packages view (#2668) * Add sorting to `Table` component (#2669) * Add sorting in `Upgradable Packages` & `Available Patches` lists (#2679) * Add header to upgradable packages view (#2685) * Add search to table component (#2684) * Add search to table component This commit adds a search capability to the table component. The search is case insensitive and respects normalises Unicode character differences. However, it is not fuzzy or can accommodate spelling mistakes. Furthermore, the change is supported via a story and several test cases, that describe the behaviour. * Functional JavaScript aka. fix ESLint Prefer array iteration methods over for loops * Implement changes proposed by Jamie Thanks for the feedback. I agree with all of it. * Ripping search out of the Table This commit removes the search API from the Table component and adds a reduced version to the @lib folder. The idea is that the components themselves are now responsible for handling search and the library just provides utilities and functions to make this simpler. * Move HostRelevantPatchesPage to new search API * Add search to UpgradablePackagesPage This commit adds search to the UpgradablePackagesPage. Furthermore, it adds a story for the page and splits the Page into a navigation and Redux independent part for easier design work. * Add inspect function This commit adds a Elixir inspired inspect function. Using this function, one can log data to the console, without breaking up long call chains. It's literally the identity function plus a "console.dir()". * Add test fro search * Add test for filtering in pages This commit adds tests for filtering the UpgradablePackagesPage and HostRelevantPatchesPage by their content via search. * Remove dead code This commit completely removes the search API from the Table. * Add search to UpgradablePackagesPage This commit adds search to the UpgradablePackagesPage. Furthermore, it adds a story for the page and splits the Page into a navigation and Redux independent part for easier design work. * Keep consistent casing * Fix case * Rename Page and UpgradablePackages * Fix ESLint * Implement changes proposed by Jamie * Add ability to download CSV file when `Download CSV` button is clicked (#2688) * SUSE Manager views navigation (#2689) * Add navigation for SUSE Manager box in host details * Fix tests and update storybook stories * Make the indicators in AvailableSoftwareUpdates a11y-compliant * Rename navigate to onNavigate * Add `Errata Details` controller (#2700) * Add controller to get errata details * Make *all* properties of response (`ErrataDetailsResponse`) snake case * Add ability to get Bugzilla fixes from SUMA (#2715) * Add advisory details page (#2699) * Add styled AdvisoryDetails page and accompanying story * Rework AdvisoryDetails for errata data This commit updates booth the AdvisoryDetails story and the component to accept data, that comes from the errata endpoint directly. * Add and mount the advisory details page This commit adds an advisory details page and mounts it. * Add factories for the advisory errata * Add simple test for the advisory details page This commit adds a test case for the advisory page. * Add AdvisoryIcon This commit adds an AdvisoryIcon component, since it has to be used in multiple places. I extracted the component, because it is easy to miss, when adding another advisory type to the backend. I can not add any a11y information to the upstream component, which means I will not be able to easily differentiate between the SVGs. I might have to do these changes upstream. * Move AdvisoryDetails and AdvisoryDetailsPage to AdvisoryIcon * Remove unneeded API and polish story * Fix typos * Adhere to snake case * Only render list element, when data is available * Fix wrong API and network call * Add render test for AdvisoryIcon * Include errata data in AdvisoryDetails test * Fix styling * Fix ESLint issues * Fix API call * Enhance backend fixture * Fix format * Add back advisory details page in the router * Fix typo add test for CVEs fixes and packages --------- Co-authored-by: Alessio Biancalana <alessio.biancalana@suse.com> --------- Co-authored-by: Jan Fooken <jan.fooken@suse.com> Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds the ability to sort the rows in a
Tablecomponent by clicking on the header of a table column.Changes to the
Tablecomponent:The
sortByprop is the predicate function used to sortdata.Columns in
config.columnscan be marked withsortable: true.Additionally, a
handleClick()function needs to be provided inconfig.columnsfor handling when the column header is clicked.Finally, in
config.columns,sortDirectionneeds to be provided to indicate to the user which column is being sorted, and in which direction. It can be'asc' | 'desc' | undefined/null. Theundefined/nullcase is for the columns not currently being sorted.Note that all these new config options have default falsy values so that when no sorting is required, none of these values need to be provided.
Some default functions to produce sorting predicates have also been provided in
assets/js/common/Table/sorting.jsHow was this tested?
Added unit test to verify that
Tablesorts data. Add Storybook story to demonstrate feature.