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

fixes i18n, new Cluster Appearance UX #10677

Merged
merged 13 commits into from
Apr 29, 2024
Merged

Conversation

scures
Copy link
Contributor

@scures scures commented Mar 21, 2024

Summary

Fixes #9810

Technical notes summary

  • Adds preview for both Menu & Cluster Header
  • Adds the new Layout (row) of the elements
  • Allows independent Abbreviation and Comment settings.
  • Allows setting the color (which will apply to both or the enable field)
  • Added some borders for badge and comment when no color is present (transparent), without, the layout would look broken.
  • Disabling the Use Custom Badge will show the auto-generated abbreviation to match the side-menu functionality.
  • Allows customization from Cluster Manager -> Cluster -> Edit when a cluster is not ready.

Areas or cases that should be tested

  • i18n should be present
  • Opening the Customise modal, the Use custom badge & Apply buttons should be disable.
  • The layout for the cluster page preview should be the real scenario
  • Not setting a custom abbreviation should not display the custom color (when a comment is set with a color)

Areas which could experience regressions

NA

Screenshot/Video

Screenshots

image
image
image
image

Screen.Recording.2024-04-25.at.08.38.44.mov

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

Signed-off-by: scures <scurescu@suse.com>
@scures scures added this to the v2.9.0 milestone Mar 21, 2024
@scures scures self-assigned this Mar 21, 2024
@scures scures requested a review from richard-cox March 21, 2024 12:46
@richard-cox richard-cox requested a review from nwmac March 22, 2024 18:08
Copy link
Member

@richard-cox richard-cox left a comment

Choose a reason for hiding this comment

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

  • User cannot customise appearance if the cluster name is less than three characters.
  • If a user only adds a cluster comment and not a custom badge .. a badge background colour is shown in the NameNsDescription component's example (but correctly not in the modal example or in the side nav when the cluster is created)
  • If a user adds a badge background colour the modal example sets the whole background as that colour instead of just the lower part

@scures
Copy link
Contributor Author

scures commented Apr 22, 2024

@richard-cox I introduce the new behavior and layout.

  • Allows independent Abbreviation and Comment
  • Allows setting the color (which will apply to both or the enable field)
  • Added some borders for badge and comment when no color is present (transparent), without, the layout would look broken.
    Before:
    image
    After:
    image
  • Implements preview for side navigation & header
  • Inlines the customs inputs & colors according to the last design.

Screenshots

image

image

@scures scures changed the title fixes i18n and small UX fixes i18n, new Cluster Appearance UX Apr 22, 2024
@scures scures requested a review from richard-cox April 22, 2024 13:49
Copy link
Member

@richard-cox richard-cox left a comment

Choose a reason for hiding this comment

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

  • the new modal layout means the images in the pr description are out of date
  • When Use a custom badge isn't selected there's no preview of the badge in RANCHER HEADER. Selecting it will show what the badge is (before icon text is entered)
  • Selecting Use a custom badge removes the existing text in the SIDE NAVIGATION preview so it's just an empty square. This contradicts reality, if the user saves changes via Apply the side menu icon remains with the existing abbreviation.
  • When Use a custom badge is unselected and Badge background color is selected, the SIDE NAVIGATION preview shows a bar at the bottom. This contradicts reality, as the bottom bar isn't shown in the side menu
  • Setting the Badge background color to white results in the bottom of the SIDE NAVIGATION preview and side menu button not showing
  • The Cluster Manager --> Cluster --> Cluster Appearance component needs to be visible on cluster edit. That is where users set it originally and where they will naturally return to change it. In addition users will not be able to get to the cluster dashboard when the cluster is not up. Not sure where we got to with this one?

shell/dialog/AddCustomBadgeDialog.vue Outdated Show resolved Hide resolved
shell/pages/c/_cluster/explorer/ConfigBadge.vue Outdated Show resolved Hide resolved
@scures
Copy link
Contributor Author

scures commented Apr 25, 2024

@richard-cox I updated the PR's description to reflect the new direction and addressed the changes from the last PR plus some other improvements.
About the last comment, I cannot find it but, we talked about this previously and agreed not to have it in there, I cannot find the conversation (still looking) but I was around most fields (cluster name, etc..) not being editable and that it would be something for later to be explored.

@scures scures requested a review from richard-cox April 25, 2024 07:01
Copy link
Member

@richard-cox richard-cox left a comment

Choose a reason for hiding this comment

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

Getting close!

  • For the local cluster clicking on Add Cluster Badge shows LCL as the side navigation icon instead of the bull head. The header icon displays this fine. Clicking on User custom badge then changes the side navigation icon to the bull, but it's black (in dark mode) instead of blue.
  • Badge background color text is duplicated (checkbox label and in grey box). It also creates whitespace between bottom text and colour picker which makes the alignment with the two controls on the left look odd. Suggest the bottom text within the grey box is removed and the colour picker is centrally aligned within grey box.
    • image
  • Menu icon borders
    • Light mode, white / light badge colour shows gap in border (see ERE badge)
      • image
    • Dark mode, black badge colour shows no border (see YAH badge). Would the solution here be to always show a border around the bottom section?
    • image
  • The Cluster Manager --> Cluster --> Cluster Appearance component needs to be visible on cluster edit. That is where users set it originally and where they will naturally return to change it. In addition users will not be able to get to the cluster dashboard when the cluster is not up.
    • About the last comment, I cannot find it but, we talked about this previously and agreed not to have it in there, I cannot find the conversation (still looking) but I was around most fields (cluster name, etc..) not being editable and that it would be something for later to be explored.

    • The name cannot be changed (it's not supported). Can you create an issue to tackle later?

shell/dialog/AddCustomBadgeDialog.vue Outdated Show resolved Hide resolved
@scures scures requested a review from richard-cox April 29, 2024 08:59
@scures scures merged commit 08f11a5 into rancher:master Apr 29, 2024
26 checks passed
@scures scures deleted the 9810/improvements branch April 29, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect app bar cluster abbreviations
2 participants