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

🐛 Fix target card behavior and rendering #2025

Merged
merged 2 commits into from
Jul 24, 2024

Conversation

sjd78
Copy link
Member

@sjd78 sjd78 commented Jul 24, 2024

Resolves: #2022
Resolves: #2010
Resolves: #1981
Resolves: #1252
Resolves: https://issues.redhat.com/browse/MTA-3094

Target card behavior and rendering on the target selection step of the analysis wizard and on the custom migrations target page have been updated:

  • All of the card contents render inside the card itself ([BUG] Custom migration target: The description textbox exceeds the target card #1981 / MTA-3094)
  • When changing the specific label selection for a target, the card selection will not changes ([BUG] Opening a version dropdown de-selects analysis target #2010)
  • The target label select now sorts the labels in numeric natural sort descending order. OpenJDK 21 will be initially selected and appear above OpenJDK 17 in the select list. ([BUG] Make OpenJDK 21 the default rather than 11 #2022)
  • Layouts used on the card have been refactored to use base layouts instead of the EmptyState component
  • The target label select box has been updated to the current SimpleSelectBasic component for current PF5 alignment
  • When card selection is enabled (on the target selection step of the analysis wizard), the selection checkbox is always displayed and the normal Card hover styles have been disabled.
  • The CardHeader.selectableActions have been aligned to current PF and the click handler moved to onChange instead of the Card.onClick. The selectableActionId needs to be unique on the page for the select handling to work properly.

Some refactoring on the SetTargets component used by the analysis wizard have been made to simplify rendering code.

Resolves: konveyor#2022
Resolves: konveyor#2010
Resolves: konveyor#1981
Resolves: https://issues.redhat.com/browse/MTA-3094

Target card behavior and rendering on the target selection
step of the analysis wizard and on the custom migrations
target page have been updated:
  - All of the card contents render inside the card itself
    (konveyor#1981 / MTA-3094)
  - When changing the specific label selection for a target,
    the card selection will not changes (konveyor#2010)
  - The target label select now sorts the labels in numeric
    natural sort descending order.  `OpenJDK 21` will be
    initially selected and appear above `OpenJDK 17` in the
    select list. (konveyor#2022)
  - Layouts used on the card have been refactored to use base
    layouts instead of the `EmptyState` component
  - The target label select box has been updated to the
    current `SimpleSelectBasic` component for current PF5
    alignment
  - When card selection is enabled (on the target selection
    step of the analysis wizard), the selection checkbox is
    always displayed and the normal `Card` hover styles have
    been disabled.

Some refactoring on the `SetTargets` component used by the
analysis wizard have been made to simplify rendering code.

Signed-off-by: Scott J Dickerson <[email protected]>
@sjd78 sjd78 requested a review from dymurray July 24, 2024 04:59
@sjd78
Copy link
Member Author

sjd78 commented Jul 24, 2024

Custom migration targets now:
screenshot-localhost_9000-2024 07 24-00_56_57

Set targets step of analysis wizard:
screenshot-localhost_9000-2024 07 24-01_00_26

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

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

Project coverage is 42.14%. Comparing base (b654645) to head (01c3c33).
Report is 199 commits behind head on main.

Files Patch % Lines
...ent/src/app/components/target-card/target-card.tsx 9.75% 37 Missing ⚠️
...pages/applications/analysis-wizard/set-targets.tsx 0.00% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2025      +/-   ##
==========================================
+ Coverage   39.20%   42.14%   +2.94%     
==========================================
  Files         146      171      +25     
  Lines        4857     5505     +648     
  Branches     1164     1376     +212     
==========================================
+ Hits         1904     2320     +416     
- Misses       2939     3069     +130     
- Partials       14      116     +102     
Flag Coverage Δ
client 42.14% <7.69%> (+2.94%) ⬆️
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.

Copy link
Member

@ibolton336 ibolton336 left a comment

Choose a reason for hiding this comment

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

Still seeing the bug outlined here when navigating back in the wizard.
#1951 (review)

Screen.Recording.2024-07-24.at.10.45.04.AM.mov

More info here: #1252 (comment)

Copy link
Member

@ibolton336 ibolton336 left a comment

Choose a reason for hiding this comment

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

Awesome job identifying that bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants