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

rszwajko · 2024-08-02T15:03:00Z

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:

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

Resolves: #2009
Resolves: https://issues.redhat.com/browse/MTA-3515

Resolves: konveyor#2009 Signed-off-by: Radoslaw Szwajkowski <[email protected]>

codecov · 2024-08-02T15:13:48Z

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 ⚠️
...src/app/components/FilterToolbar/FilterToolbar.tsx	0.00%	1 Missing ⚠️
client/src/app/queries/targets.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            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	Coverage Δ
client	`42.53% <17.24%> (+3.33%)`	⬆️
server	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Track success status of both the `targets` and `tagCategories` queries - When they are both successful, then the pre-selection of targets providers based on application language tags can be done properly. - Use a `useEffect()` to do this here. It mostly works but will reset the selection to default if any of the related queries refetch. (Switch browser tabs and the fetches will run again when the application become visible again) Signed-off-by: Scott J Dickerson <[email protected]>

sjd78

There are a few things that could be done a bit different, but the biggest issue is actually when the initial selection is made. The targets and tag catagories may not have landed yet...and that makes the UI look strange for a few moments.

client/src/app/pages/applications/analysis-wizard/set-targets.tsx

Additional fetaures: 1. pre-fetch targets and tag categories to pre-filter targets 2. add error and loading handling Signed-off-by: Radoslaw Szwajkowski <[email protected]>

rszwajko · 2024-08-07T19:39:00Z

@sjd78
Changes:

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

rszwajko · 2024-08-07T19:45:31Z

@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:

re-enable useSetting("ui.target.order")
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 oVirt/ovirt-web-ui#1600

Assume the setting is optional and do not block the flow if missing. Signed-off-by: Radoslaw Szwajkowski <[email protected]>

…ter toolbar Signed-off-by: Radoslaw Szwajkowski <[email protected]>

sjd78

Looking really good. Mostly comments in the data hook.

client/src/app/pages/applications/analysis-wizard/set-targets.tsx

Signed-off-by: Radoslaw Szwajkowski <[email protected]>

rszwajko · 2024-08-09T19:07:50Z

Optional queries failed during initial load

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

custom ordering
pre-filtering based on discovered language tags

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

Screencast.from.2024-08-09.20-41-50.mp4

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.

Screencast.from.2024-08-09.20-40-54.mp4

Signed-off-by: Radoslaw Szwajkowski <[email protected]>

rszwajko · 2024-08-13T11:27:58Z

@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.

sjd78

LGTM

client/src/app/pages/applications/analysis-wizard/set-targets.tsx

JustinXHale · 2024-08-13T21:37:09Z

@sjd78 @rszwajko LGTM!

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: #2009 Resolves: https://issues.redhat.com/browse/MTA-3515 --------- Signed-off-by: Radoslaw Szwajkowski <[email protected]> Signed-off-by: Scott J Dickerson <[email protected]> Co-authored-by: Scott J Dickerson <[email protected]> Signed-off-by: Cherry Picker <[email protected]>

…r#2045) 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: konveyor#2009 Resolves: https://issues.redhat.com/browse/MTA-3515 --------- Signed-off-by: Radoslaw Szwajkowski <[email protected]> Signed-off-by: Scott J Dickerson <[email protected]> Co-authored-by: Scott J Dickerson <[email protected]>

…2055) 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: #2009 Resolves: https://issues.redhat.com/browse/MTA-3515 --------- Signed-off-by: Radoslaw Szwajkowski <[email protected]> Signed-off-by: Scott J Dickerson <[email protected]> Co-authored-by: Radoslaw Szwajkowski <[email protected]> Co-authored-by: Scott J Dickerson <[email protected]> Signed-off-by: Cherry Picker <[email protected]>

rszwajko requested review from ibolton336 and sjd78 as code owners August 2, 2024 15:03

Choose initially selected languages from available providers

5052390

Resolves: konveyor#2009 Signed-off-by: Radoslaw Szwajkowski <[email protected]>

rszwajko force-pushed the extraLang branch from 09e54c7 to 5052390 Compare August 2, 2024 15:06

Merge branch 'main' into extraLang

703451d

sjd78 added this to the v0.5.1 milestone Aug 5, 2024

sjd78 added the cherry-pick/release-0.5 This PR should be cherry-picked to release-0.5 branch. label Aug 5, 2024

sjd78 reviewed Aug 5, 2024

View reviewed changes

sjd78 modified the milestones: v0.5.1, v0.5.2 Aug 7, 2024

Use table hooks to provide target filtering

41a6042

Additional fetaures: 1. pre-fetch targets and tag categories to pre-filter targets 2. add error and loading handling Signed-off-by: Radoslaw Szwajkowski <[email protected]>

rszwajko added 3 commits August 8, 2024 16:47

Pre-load setting ui.target.order

69efaca

Assume the setting is optional and do not block the flow if missing. Signed-off-by: Radoslaw Szwajkowski <[email protected]>

Merge branch 'main' into extraLang

39879ca

Disable paging and use less restrictive breakpoint for compacting fil…

2022860

…ter toolbar Signed-off-by: Radoslaw Szwajkowski <[email protected]>

sjd78 requested changes Aug 9, 2024

View reviewed changes

Handle network failures during initial data load

fdab75b

Signed-off-by: Radoslaw Szwajkowski <[email protected]>

Keep stable references for data returned from the hook

da7785e

Signed-off-by: Radoslaw Szwajkowski <[email protected]>

rszwajko changed the title ~~🐛 Choose initially selected languages from available providers~~ 🐛 Use standard filters for languages in the Set Targets step Aug 13, 2024

rszwajko requested a review from sjd78 August 13, 2024 11:23

sjd78 approved these changes Aug 13, 2024

View reviewed changes

client/src/app/pages/applications/analysis-wizard/set-targets.tsx Show resolved Hide resolved

sjd78 requested a review from JustinXHale August 13, 2024 19:36

Merge branch 'main' into extraLang

ff312e6

rszwajko merged commit ab89b15 into konveyor:main Aug 14, 2024
12 checks passed

This was referenced Oct 1, 2024

operator konveyor-operator (0.6.0-alpha.1) k8s-operatorhub/community-operators#5094

Merged

operator konveyor-operator (0.6.0-alpha.1) redhat-openshift-ecosystem/community-operators-prod#5272

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

rszwajko commented Aug 2, 2024 •

edited

Loading

codecov bot commented Aug 2, 2024 •

edited

Loading

sjd78 left a comment

rszwajko commented Aug 7, 2024

rszwajko commented Aug 7, 2024

sjd78 left a comment

rszwajko commented Aug 9, 2024 •

edited

Loading

rszwajko commented Aug 13, 2024

sjd78 left a comment

JustinXHale commented Aug 13, 2024

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

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

Conversation

rszwajko commented Aug 2, 2024 • edited Loading

codecov bot commented Aug 2, 2024 • edited Loading

Codecov Report

sjd78 left a comment

Choose a reason for hiding this comment

rszwajko commented Aug 7, 2024

rszwajko commented Aug 7, 2024

sjd78 left a comment

Choose a reason for hiding this comment

rszwajko commented Aug 9, 2024 • edited Loading

Optional queries failed during initial load

Required query failed

rszwajko commented Aug 13, 2024

sjd78 left a comment

Choose a reason for hiding this comment

JustinXHale commented Aug 13, 2024

rszwajko commented Aug 2, 2024 •

edited

Loading

codecov bot commented Aug 2, 2024 •

edited

Loading

rszwajko commented Aug 9, 2024 •

edited

Loading