konveyor / tackle2-ui

Tackle (2nd generation) UI component.
Apache License 2.0
8 stars 43 forks source link

:bug: Use standard filters for languages in the Set Targets step #2045

Closed rszwajko closed 3 months ago

rszwajko commented 3 months ago

Before, in the Set Targets step, if the languages were not pre-selected automatically then no cards were visible. In order to see target cards user had to select at least one language. After this fix, all target cards will be visible.

Additional changes:

  1. wait (display a spinner) until all necessary queries finished before displaying Set Targets step
  2. improve error handling
    1. display error when targets could not be loaded
    2. silently ignore failed optional queries and continue without extra featues
  3. extract data fetching into one hook

Resolves: https://github.com/konveyor/tackle2-ui/issues/2009 Resolves: https://issues.redhat.com/browse/MTA-3515

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 17.24138% with 48 lines in your changes missing coverage. Please review.

Project coverage is 42.53%. Comparing base (b654645) to head (ff312e6). Report is 213 commits behind head on main.

Files Patch % Lines
...pages/applications/analysis-wizard/set-targets.tsx 17.85% 45 Missing and 1 partial :warning:
...src/app/components/FilterToolbar/FilterToolbar.tsx 0.00% 1 Missing :warning:
client/src/app/queries/targets.ts 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2045 +/- ## ========================================== + Coverage 39.20% 42.53% +3.33% ========================================== Files 146 171 +25 Lines 4857 5504 +647 Branches 1164 1370 +206 ========================================== + Hits 1904 2341 +437 - Misses 2939 3046 +107 - Partials 14 117 +103 ``` | [Flag](https://app.codecov.io/gh/konveyor/tackle2-ui/pull/2045/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=konveyor) | Coverage Δ | | |---|---|---| | [client](https://app.codecov.io/gh/konveyor/tackle2-ui/pull/2045/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=konveyor) | `42.53% <17.24%> (+3.33%)` | :arrow_up: | | [server](https://app.codecov.io/gh/konveyor/tackle2-ui/pull/2045/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=konveyor) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=konveyor#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rszwajko commented 3 months ago

@sjd78 Changes:

  1. improved error handling (but based on wrapper that "waits" for data )
  2. filtering instead of selecting targets - based on feedback from https://issues.redhat.com/browse/MTA-3515

image

rszwajko commented 3 months ago

@sjd78 @ibolton336 In the last commit I've managed to use our almighty table hooks to power the card view. This brings us closer to PF4 Card View pattern. Missing parts:

  1. re-enable useSetting("ui.target.order")
  2. verify if we need the form to store the selection - the hook could handle that in similar way as in the table case

Fun fact ....it's not the first time when I'm moving from card to table view :) Check out https://github.com/oVirt/ovirt-web-ui/pull/1600

rszwajko commented 3 months ago

Optional queries failed during initial load

The step loads correctly but lacks the features that depend on the optional queries:

  1. custom ordering
  2. pre-filtering based on discovered language tags

After both queries are unblocked the screen loads the missing features.

https://github.com/user-attachments/assets/49671add-2386-410b-b06a-a4899fce903a

Required query failed

Without the targets the step has no data to work on - however the user still may skip the step and continue to the next one.

https://github.com/user-attachments/assets/e4e8d77d-787d-494a-b6b3-bb7a5b410201

rszwajko commented 3 months ago

@JustinXHale please take a look at screenshots above - basically we are moving towards standard Card View which means that we can add more filters or other features (sorting, paging, actions). We can also add a toggle that would allow users to switch to table or list view.

JustinXHale commented 3 months ago

@sjd78 @rszwajko LGTM!