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

Add search to table component #2684

Merged
merged 17 commits into from
Jun 13, 2024
Merged

Add search to table component #2684

merged 17 commits into from
Jun 13, 2024

Conversation

janvhs
Copy link
Member

@janvhs janvhs commented Jun 10, 2024

Description

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.

Tests

Furthermore, the change is supported via a story and several test cases, that describe the behaviour.

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.
@janvhs janvhs added enhancement New feature or request javascript Pull requests that update Javascript code labels Jun 10, 2024
Prefer array iteration methods over for loops
@jamie-suse
Copy link
Contributor

I'd add a bit more semantic meaning to the variable names in search.js, will add some suggestions.

Thanks for the feedback. I agree with all of it.
Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

Good job! Besides @jamie-suse's comments I'd like if possible to see if we could have something using the filtering on the raw data instead of embedding this into the table component. Could you do that?

@janvhs
Copy link
Member Author

janvhs commented Jun 10, 2024

@dottorblaster Yeah, I agree with handling the filtering outside the table component, since the Table feels really really bloated anyway

janvhs added 4 commits June 13, 2024 09:57
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.
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.
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()".
@janvhs janvhs marked this pull request as draft June 13, 2024 08:12
@janvhs
Copy link
Member Author

janvhs commented Jun 13, 2024

@dottorblaster @jamie-suse This is basically done. I just need to port the search tests from the table to the individual pages, but if you want to look at this, you could :D

Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

Very good job, I think you also have to get rid of search.js inside Table. Keep going!

janvhs added 2 commits June 13, 2024 14:11
This commit adds tests for filtering the UpgradablePackagesPage and
HostRelevantPatchesPage by their content via search.
This commit completely removes the search API from the Table.
@janvhs janvhs marked this pull request as ready for review June 13, 2024 12:16
@@ -86,3 +86,8 @@ export async function recordSaga(saga, initialAction, state = {}) {

return dispatched;
}

export const inspect = (val) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this used anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, but I used it for debugging long call chains a lot. I thought it might be useful to have it in the test utils

janvhs added 2 commits June 13, 2024 16:03
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.
@@ -1,3 +1,3 @@
import UpgradablePackagesPage from './UpgradablePackagesPage';

export default UpgradablePackagesPage;
export default UpgradablePackagesPage
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should fail the lint step?

Copy link
Contributor

Choose a reason for hiding this comment

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

grrr reactions for out prettier check, why did it green light this? :0

This comment was marked as outdated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Prettier should be really mad for this, I don't why it doesn't happen

@janvhs
Copy link
Member Author

janvhs commented Jun 13, 2024

Thank you for the thorough and really, really helpful review @jamie-suse . I really appreciate it! :D

export default function UpgradablePackages({ hostName, upgradablePackages }) {
const [search, setSearch] = useState('');

const displayedPackages = upgradablePackages.filter(
Copy link
Contributor

Choose a reason for hiding this comment

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

nice 😎

@janvhs janvhs merged commit eaa1e56 into suse-manager-overviews Jun 13, 2024
26 checks passed
@janvhs janvhs deleted the search branch June 13, 2024 16:31
dottorblaster pushed a commit that referenced this pull request Jun 27, 2024
* 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
dottorblaster pushed a commit that referenced this pull request Jun 28, 2024
* 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
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
Labels
enhancement New feature or request javascript Pull requests that update Javascript code
Development

Successfully merging this pull request may close these issues.

3 participants