Resolve remaining issues from UI review

[thatblindgeye]

Goes towards closing the following tickets:

• RHOAIENG-16933
• RHOAIENG-16934
• RHOAIENG-16938
• RHOAIENG-16939
• RHOAIENG-16940
• RHOAIENG-16942
• RHOAIENG-16944
• RHOAIENG-16946
• RHOAIENG-16947
• RHOAIENG-16950
• RHOAIENG-17072

Description

Goes towards resolving remaining issues found during the designer UI review from the original PF6 migration PR.

• Added custom styling to add underline decoration to links
• Updated labels in the "Model registry settings" page to use stateful variants rather than color, except for when the label is "degrading" in which case a non-status gray is used.
• Updated where the dropdown is appended as well as its max height in the "autofill from data connection" form.
• For https://issues.redhat.com/browse/RHOAIENG-16939, removed custom styling for height, min-width and padding and applied a max-width of 530px. This mimics the result as if removing the width 100% on the chart svg itself, and also provides some responsiveness to the chart.
• Updated deployed model cards to be the same height even when one has more content.
• Updating spacing to be consistent on pages with toolbars and tables.
  
• Updated the alignment of footer actions in the "Register model" form to align left with the form content.
• Replaced the Tooltip with a Popover on the "Requested resources" content + updated to use a button for the trigger.
• Removed flex wrap on "Applications > Explore" and "Resources" pages (card view for the latter) to prevent unnecessary scrolling. Go to aforementioned pages and adjust the viewport width; a scrollbar should only appear when necessary, e.g. when the cards begin to wrap, and should not have significant empty space at the bottom of the card content area.
• Updated styling on "Input parameters" tab of an experiment view to allow scrolling; previously multiple code editor content would be cut off.
• Adjusted the styling for the help button on the "Settings > Storage classes" page inside table rows to have content align within the row. Note that this is using custom styling to match other instances of this help button in other tables, and removing that custom styling across all uses is something that can be revisited in the future. The content should look like the following screenshot:
  
• Applied similar styling to other brand logos to the Jupyter logo seen on the home page in the "Looking for the previous landing page?" section (white background with rounded corners)

How Has This Been Tested?

Visually tested in local build -- page availability permitting. Otherwise using the current v6 cluster tested solutions in dev tools and ported those fixes to the codebase.

• Go to the "Experiments and runs" page and check that the project titles in the "Experiment" column are underlined and not bold. Additionally on the home page, the cards in the "Data Science Projects" section should have underlined links.
• Go to "Settings > Model registry settings" and ensure that the labels being used in the "Status" column of the table are status variant rather than color variants (e.g. pf-m-warning instead of pf-m-orangered etc). The only exception to this is the "degrading" status, which should use the grey color still.
• Go to "Modal registry" page, click the "Register model" button, scroll and click the "Autofill from data connection" button, then open the "Project" select. The menu should break out of the modal and no menu content should be cut off by the bottom of the viewport
• Go to "Distributed Workload Metrics" and then the "Distributed workload status" tab (I've been using "KunTestPF6" project). Ensure that the chart in the "Overview" card is not overly large (the text of the chart legend should be about the same size as the "Overview" card title, give or take).
• Go to Data Science Projects and click a project and on its overview page go to the "Deployed models" section. Cards should be of the same height -- may require manually adjusting the content in the cards in dev tools to see them retain the same height despite the difference in amount of content
• Compare the Data Science Projects page with the various pages under the Settings sidenav and ensure the spacing between the title/description + toolbar and toolbar + table is consistent.
• In the "Model registry" page, ensure that the footer actions (i.e. "Register ...." and "Cancel" buttons) are aligned in a way that matches the following screenshot. This should be checked on the form from clicking "Register model", clicking the caret icon in that "Register model" button then clicking "Register new version", and clicking a model name in the table on the "Model registry" page then clicking the "Register new version" button (3 views total).
  
• Go to "Distributed Workload Metrics" page and the Card with the "Requested resources" title should now have a Popover instead (triggering it will open content with a close button in the top right).
• Go to both "Applications > Explore" and "Resources" pages (card view for the latter) and adjust the viewport width; a scrollbar should only appear when necessary, e.g. when the cards begin to wrap, and should not have significant empty space at the bottom of the card content area.
• Go to "Experiments and runs" page, select "Yan's UI review testing" from the project selector > click "Default" experiment > click one of the runs named "simtest..." > go to "Input parameters" tab. The page should be scrollable to allow viewing all content on the page.
  • Additionally, go to the "Pipeline spec" tab on this same page and ensure that the code editor content does not flow outside of its container borders.
• Go to "Settings > Storage classes" and ensure the content in table rows aligns more evenly, as is seen in the following screenshot:
  
• On the homepage in the "Looking for the previous landing page?" section at the top of the page, ensure the Jupyter logo has styling that matches brand logos on the "Applications > Explore" page and that the logo is perceivable in both light and dark theme. As shown in following screenshot:
  

Test Impact

Request review criteria:

Self checklist (all need to be checked):

• The developer has manually tested the changes and verified that the changes work
• Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• The developer has added tests or explained why testing cannot be added (unit or cypress tests for related changes)

If you have UI changes:

• Included any necessary screenshots or gifs if it was a UI change.
• Included tags to the UX team if it was a UI/UX change.

After the PR is posted & before it merges:

• The developer has tested their solution on a cluster by using the image produced by the PR to main

[openshift-ci]

Hi @thatblindgeye. Thanks for your PR.

I'm waiting for a opendatahub-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[thatblindgeye]

/cc @jeff-phillips-18

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.18%. Comparing base (14b1c4d) to head (a0891b3).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3578      +/-   ##
==========================================
+ Coverage   85.16%   85.18%   +0.02%     
==========================================
  Files        1393     1393              
  Lines       31939    31936       -3     
  Branches     8955     8954       -1     
==========================================
+ Hits        27200    27204       +4     
+ Misses       4739     4732       -7     

Files with missing lines	Coverage Δ
...d/src/components/searchSelector/SearchSelector.tsx	84.21% <100.00%> (ø)
frontend/src/components/table/TableBase.tsx	100.00% <100.00%> (ø)
.../src/components/table/TableRowTitleDescription.tsx	100.00% <ø> (ø)
...nd/src/concepts/dashboard/DashboardHelpTooltip.tsx	100.00% <100.00%> (ø)
...cepts/dashboard/codeEditor/DashboardCodeEditor.tsx	100.00% <100.00%> (ø)
...s/content/pipelinesDetails/PipelineDetailsYAML.tsx	91.66% <ø> (ø)
...tent/pipelinesDetails/pipeline/PipelineDetails.tsx	85.24% <ø> (ø)
...inesDetails/pipelineRun/PipelineRunDetailsTabs.tsx	100.00% <ø> (ø)
...s/content/tables/experiment/ExperimentTableRow.tsx	100.00% <ø> (ø)
frontend/src/pages/ApplicationsPage.tsx	97.72% <ø> (ø)
... and 19 more

... and 5 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14b1c4d...a0891b3. Read the comment docs.

[jeff-phillips-18]

/ok-to-test

[thatblindgeye]

Just to note, all of the remaining issues from the original ui review Google doc have been marked as confirmed by UX (exception being one that has a separate Jira ticket, the icon background colors in dark theme). cc @simrandhaliw

[jeff-phillips-18]

In Mode Registry Settings the Manage permissions action is mis-aligned:

[simrandhaliw]

Except for the issue that @jeff-phillips-18 also pointed out, LGTM!

[thatblindgeye]

FWIW, this is probably the expected styling coming out of PF. Comparing the view on this page in the dashboard vs from a PF example I updated to try and match:

Dashboard

PatternFly

The button being used - when it's not hovered/focused - does make it look off, but everything is aligned along the top of each element

[jeff-phillips-18]

@thatblindgeye OK, we can move forward regardless of the misalignment. If there is a PF issue logged please link it here. We should also log an issue to fix it locally.

[jeff-phillips-18]

If we are changing the default to false we can probably remove the prop all together and update any instances that explicitly set false to remove the setting. Are there any that explicitly want it to be true?

[thatblindgeye]

Sim and I had gone over this when we met earlier this week. I had mentioned either removing this completely or updating the default value, and I had mentioned updating the default value in case - for whatever reason - a bold title was desired in the future. I have no problem removing the prop altogether though if that's preferred.

[jeff-phillips-18]

The default is false

Suggested change

boldTitle={false}

[thatblindgeye]

Ah yep, the update to the default was done after this update and I forgot to remove this. Will go back through to clean up this prop usage.

[jeff-phillips-18]

RHOAIENG-16950 appears to still be an issue

[thatblindgeye]

OK, we can move forward regardless of the misalignment. If there is a PF issue logged please link it here. We should also log an issue to fix it locally.

patternfly/patternfly#7267

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jeff-phillips-18
Once this PR has been reviewed and has the lgtm label, please assign manosnoam for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• frontend/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

New changes are detected. LGTM label has been removed.