Support better K8s Resource Name editing #3167

andrewballantyne · 2024-09-06T20:19:26Z

https://issues.redhat.com/browse/RHOAIENG-583

Description

Provides a new cleaner interface to manage DisplayNames & Descriptions & K8s Name generation. This component will provide a unified front for any element we need to manage K8s items with.

ONLY REFACTORS Project modal & Workbench form. There will be more that can use this.

Screenshots

Project Modal:

Click to expand more Project Screenshots

These are in chronological order of operation.

Initial State:

Just Display Name Added:

Looking at the resource name:

Make the display name too long:

Make the resource name exceed max characters:

Throw on an invalid character:

Trim it back to the max (no auto trimmed action, so no warning):

Clear out the resource name:

Make an invalid k8s name that is short:

Make it a valid k8s name:

After submit & editing:

Workbench screen (only the workbench name is configured this way -- other resources created here are not YET):

How Has This Been Tested?

Using the Create/Edit Project Modals & the Create/Edit Workbench Forms.

Test Impact

Added unit tests for the new component. Created a Cypress Page object and refactored the Project/Workbench page objects to use it. Existing Cypress tests should cover the Project modal & Workbench form; minor adjustments made for new design.

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

frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx

codecov · 2024-09-08T21:05:03Z

Codecov Report

Attention: Patch coverage is 92.85714% with 12 lines in your changes missing coverage. Please review.

Project coverage is 85.36%. Comparing base (f046e94) to head (42c5bd3).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...ages/projects/screens/projects/ProjectListView.tsx	9.09%	10 Missing ⚠️
...ges/projects/screens/projects/NewProjectButton.tsx	80.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3167      +/-   ##
==========================================
+ Coverage   85.32%   85.36%   +0.03%     
==========================================
  Files        1270     1275       +5     
  Lines       27900    28010     +110     
  Branches     7422     7449      +27     
==========================================
+ Hits        23807    23912     +105     
- Misses       4093     4098       +5

Files with missing lines	Coverage Δ
frontend/src/api/k8s/notebooks.ts	`97.45% <100.00%> (-0.03%)`	⬇️
...K8sNameDescriptionField/HelperTextItemVariants.tsx	`100.00% <100.00%> (ø)`
...8sNameDescriptionField/K8sNameDescriptionField.tsx	`100.00% <100.00%> (ø)`
.../k8s/K8sNameDescriptionField/ResourceNameField.tsx	`100.00% <100.00%> (ø)`
.../src/concepts/k8s/K8sNameDescriptionField/utils.ts	`100.00% <100.00%> (ø)`
frontend/src/concepts/k8s/NameDescriptionField.tsx	`92.50% <ø> (-2.96%)`	⬇️
...src/concepts/k8s/ResourceNameDefinitionTooltip.tsx	`100.00% <100.00%> (ø)`
frontend/src/concepts/k8s/utils.ts	`100.00% <100.00%> (ø)`
...ontend/src/pages/home/projects/ProjectsSection.tsx	`93.02% <100.00%> (ø)`
...s/projects/screens/projects/ManageProjectModal.tsx	`77.50% <100.00%> (-4.86%)`	⬇️
... and 6 more

... and 4 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 f046e94...42c5bd3. Read the comment docs.

andrewballantyne · 2024-09-09T12:32:22Z

@xianli123 @kywalker-rh Please review the screenshots (they are mostly in a collapsed section, look for the expand text in the screenshot section of the description and expand it)

xianli123 · 2024-09-09T15:27:02Z

Thanks @andrewballantyne . It looks great.

There is one point we could discuss, and I have attached a graph below.
Is it possible that the input automatically truncates to the first 30 characters when the user inputs more than 30 characters? At this moment, the warning message is displayed below the input field. In this way, it could enhance the user experience by eliminating the need for users to manually remove excess characters. What do you think? cc @kywalker-rh

andrewballantyne · 2024-09-09T15:35:35Z

@xianli123 I don't understand the desire -- why don't we limit bad characters... why even have error states? I feel the feedback is most commonly done via when they manually type into the field not to "stop them" but to inform them?

(started a thread on slack -- there will be more questions about this direction)

kywalker-rh · 2024-09-09T19:30:18Z

Reviewed this with @andrewballantyne in the slack thread. Our decision is to remove the yellow warning text when the resource name has been truncated. Once that change is made this lgtm. cc: @xianli123

andrewballantyne · 2024-09-09T20:59:53Z

Changes needed to page objects & UX implementation

xianli123 · 2024-09-10T02:59:44Z

I agree with you on the update. @kywalker-rh. After the change, it will LGTM.

andrewballantyne · 2024-09-11T18:37:05Z

frontend/src/__tests__/cypress/cypress/pages/components/subComponents/SubComponentBase.ts

Dupe implementation from #3172

andrewballantyne · 2024-09-11T18:41:34Z

@xianli123 @kywalker-rh Okay, updated the screenshot in the description.

New state when you type a too long display name (and we auto translate it for k8s):

frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx

kywalker-rh · 2024-09-11T19:57:28Z

Looks great, thanks @andrewballantyne!

xianli123 · 2024-09-12T02:27:48Z

LGTM! Thanks @andrewballantyne .

frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx

frontend/src/concepts/k8s/K8sNameDescriptionField/utils.ts

frontend/src/concepts/k8s/NameDescriptionField.tsx

DaoDaoNoCode · 2024-09-12T18:18:07Z

frontend/src/concepts/k8s/K8sNameDescriptionField/ResourceNameField.tsx

+  return (
+    <StackItem>
+      <FormGroup {...formGroupProps} isRequired>
+        <TextInput


Do you need id={`${dataTestId}-resourceName`} here?

Good catch! I had to swing back through with AXE support after I built this all from the ground up. I'll make that change 👍

DaoDaoNoCode · 2024-09-12T18:35:33Z

I found an incorrect helper text condition. The second helper text should be an error in this situation.

Reproduce:
Don't touch the k8s name and input the display name as - (or anything ends with -)
The form is not able to be submitted, which is good because the name is invalid, we just need to update the logic to show the second helper text as an error state.

DaoDaoNoCode · 2024-09-12T18:43:03Z

frontend/src/concepts/k8s/K8sNameDescriptionField/utils.ts

+            safeK8sPrefix: safePrefix,
+          });
+          changedData.k8sName = {
+            value: k8sValue,


I think you might need to add a check for invalidCharacters here.

andrewballantyne · 2024-09-16T14:45:20Z

Last commit addresses the special character situation. I also rebased to get rid of the conflict

DaoDaoNoCode

Nice handling with most of the edge cases we can find so far.
/lgtm

openshift-ci · 2024-09-16T18:56:45Z

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: DaoDaoNoCode

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

The pull request process is described 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 bot added the do-not-merge/work-in-progress This PR is in WIP state label Sep 6, 2024

openshift-ci bot requested review from alexcreasy and DaoDaoNoCode September 6, 2024 20:19

andrewballantyne force-pushed the support-better-resource-name branch from 6364a1b to 562adfb Compare September 6, 2024 20:25

christianvogt reviewed Sep 6, 2024

View reviewed changes

frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx Outdated Show resolved Hide resolved

andrewballantyne force-pushed the support-better-resource-name branch 4 times, most recently from d375790 to 66eb345 Compare September 8, 2024 20:25

andrewballantyne changed the title ~~[WIP] Support better K8s Resource Name editing~~ Support better K8s Resource Name editing Sep 9, 2024

openshift-ci bot removed the do-not-merge/work-in-progress This PR is in WIP state label Sep 9, 2024

andrewballantyne changed the title ~~Support better K8s Resource Name editing~~ [WIP] Support better K8s Resource Name editing Sep 9, 2024

openshift-ci bot added the do-not-merge/work-in-progress This PR is in WIP state label Sep 9, 2024

andrewballantyne mentioned this pull request Sep 11, 2024

Refactor the project dropdown #3172

Merged

6 tasks

andrewballantyne force-pushed the support-better-resource-name branch from 36aed1b to 8cd1751 Compare September 11, 2024 18:35

andrewballantyne commented Sep 11, 2024

View reviewed changes

andrewballantyne changed the title ~~[WIP] Support better K8s Resource Name editing~~ Support better K8s Resource Name editing Sep 11, 2024

openshift-ci bot removed the do-not-merge/work-in-progress This PR is in WIP state label Sep 11, 2024

christianvogt reviewed Sep 11, 2024

View reviewed changes

frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx Outdated Show resolved Hide resolved

Gkrumbach07 reviewed Sep 12, 2024

View reviewed changes

frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx Show resolved Hide resolved

frontend/src/concepts/k8s/K8sNameDescriptionField/utils.ts Show resolved Hide resolved

frontend/src/concepts/k8s/NameDescriptionField.tsx Show resolved Hide resolved

DaoDaoNoCode reviewed Sep 12, 2024

View reviewed changes

DaoDaoNoCode mentioned this pull request Sep 13, 2024

Improve update call in editing workbench #3168

Merged

6 tasks

andrewballantyne added 8 commits September 16, 2024 10:43

Support better K8s Resource Name editing

2832a83

Remove AutoFocus hook; use native react autofocus

2930c37

Align to existing infrastructure around annotations

6fad0be

Support AXE better

1999533

Add Cypress page object for the component

6b2a0e4

Improve CodeCov for new components

6619967

Drop autoTrimmed based on UX feedback & update tests to match new design

4a70262

Support read-only mode & improve updating data

6c21e75

andrewballantyne force-pushed the support-better-resource-name branch from 61d7d0b to 69ed014 Compare September 16, 2024 14:44

andrewballantyne requested review from DaoDaoNoCode and Gkrumbach07 September 16, 2024 14:45

Handle special character cases

1bbfe88

andrewballantyne force-pushed the support-better-resource-name branch from 69ed014 to 1bbfe88 Compare September 16, 2024 14:53

andrewballantyne added 3 commits September 16, 2024 13:54

Address leading dashes & safeK8sPrefix clash

0da840c

Improved test examples for extreme edge cases & documentation of flags

0c5afdb

Handle additional edge cases

42c5bd3

DaoDaoNoCode approved these changes Sep 16, 2024

View reviewed changes

openshift-ci bot assigned DaoDaoNoCode Sep 16, 2024

openshift-ci bot added the lgtm label Sep 16, 2024

andrewballantyne added the approved label Sep 16, 2024

openshift-merge-bot bot merged commit 7964daa into opendatahub-io:main Sep 16, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support better K8s Resource Name editing #3167

Support better K8s Resource Name editing #3167

andrewballantyne commented Sep 6, 2024 •

edited

Loading

codecov bot commented Sep 8, 2024 •

edited

Loading

andrewballantyne commented Sep 9, 2024 •

edited

Loading

xianli123 commented Sep 9, 2024

andrewballantyne commented Sep 9, 2024 •

edited

Loading

kywalker-rh commented Sep 9, 2024

andrewballantyne commented Sep 9, 2024

xianli123 commented Sep 10, 2024

andrewballantyne Sep 11, 2024

andrewballantyne commented Sep 11, 2024

kywalker-rh commented Sep 11, 2024

xianli123 commented Sep 12, 2024

DaoDaoNoCode Sep 12, 2024

andrewballantyne Sep 12, 2024

DaoDaoNoCode commented Sep 12, 2024

DaoDaoNoCode Sep 12, 2024

andrewballantyne commented Sep 16, 2024

DaoDaoNoCode left a comment

openshift-ci bot commented Sep 16, 2024

Support better K8s Resource Name editing #3167

Support better K8s Resource Name editing #3167

Conversation

andrewballantyne commented Sep 6, 2024 • edited Loading

Description

Screenshots

How Has This Been Tested?

Test Impact

Request review criteria:

codecov bot commented Sep 8, 2024 • edited Loading

Codecov Report

andrewballantyne commented Sep 9, 2024 • edited Loading

xianli123 commented Sep 9, 2024

andrewballantyne commented Sep 9, 2024 • edited Loading

kywalker-rh commented Sep 9, 2024

andrewballantyne commented Sep 9, 2024

xianli123 commented Sep 10, 2024

andrewballantyne Sep 11, 2024

Choose a reason for hiding this comment

andrewballantyne commented Sep 11, 2024

kywalker-rh commented Sep 11, 2024

xianli123 commented Sep 12, 2024

DaoDaoNoCode Sep 12, 2024

Choose a reason for hiding this comment

andrewballantyne Sep 12, 2024

Choose a reason for hiding this comment

DaoDaoNoCode commented Sep 12, 2024

DaoDaoNoCode Sep 12, 2024

Choose a reason for hiding this comment

andrewballantyne commented Sep 16, 2024

DaoDaoNoCode left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Sep 16, 2024

andrewballantyne commented Sep 6, 2024 •

edited

Loading

codecov bot commented Sep 8, 2024 •

edited

Loading

andrewballantyne commented Sep 9, 2024 •

edited

Loading

andrewballantyne commented Sep 9, 2024 •

edited

Loading