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

Custom banner header without fontSize breaks top navigation #10140

Closed
braunsonm opened this issue Dec 12, 2023 · 7 comments · Fixed by #10242
Closed

Custom banner header without fontSize breaks top navigation #10140

braunsonm opened this issue Dec 12, 2023 · 7 comments · Fixed by #10242
Assignees
Labels
Milestone

Comments

@braunsonm
Copy link

Setup

  • Rancher version: v2.8.0
  • Rancher UI Extensions: None
  • Browser type & version: Chrome

Describe the bug
In Rancher 2.7.9, if you customized your UI to have a custom header banner, the fontSize is not a required field. This worked fine and it fell back to the default of 14px.

In Rancher 2.8.0, fontSize not being set will result in the entire Vue component failing to render, breaking navigation.

The error comes from this line: https://github.com/rancher/dashboard/blob/master/shell/components/nav/TopLevelMenu.vue#L79
which will be undefined if the fontSize is not set.

To Reproduce
Set a custom header banner without a fontSize defined.

Result
Your navigation will no longer render and you will see console errors.

Expected Result
The navbar should work with a fallback default fontSize

@braunsonm
Copy link
Author

Seems like the UI configuration JSON also changed the key from banner to bannerHeader in 2.8.0. Existing configurations will be broken in other ways if a migration doesn't take place (didn't in my case).

@aalves08 aalves08 self-assigned this Dec 27, 2023
@aalves08
Copy link
Contributor

@braunsonm I cannot reproduce this in v2.8.0. Can you send the me that old JSON structure, please? Also, did you update Rancher recently? Was it from v2.7.9 to v2.8.0?

How did you achieve the "setting" of that fontSize to a null, for example? We cannot change the Select on the Banners configuration for it to be a null.

I had to hack the network request to mimic that and even with that, I couldn't reproduce this.

Thanks in advance

@braunsonm
Copy link
Author

The old, broken, JSON structure is

{
  "banner": {
    "color": "#78c9cf",
    "background": "#27292e",
    "text": "Hello World!"
  },
  "showHeader": "true",
  "showFooter": "false"
}

@aalves08

@richard-cox
Copy link
Member

/backport v2.8.next1

@richard-cox
Copy link
Member

Workaround details in #10368 (comment)

@nwmac nwmac added the automation-candidate Candidate for being covered by automated tests label Mar 4, 2024
@aalves08
Copy link
Contributor

aalves08 commented Apr 2, 2024

Removing the label automation-candidate and remove the label QA-manual-test because there's a unit test that covers this scenario, introduced by https://github.com/rancher/dashboard/pull/10242/files#diff-141a080e9f374303d0363dc0a050d25097537505821af46c93df538c103031c1

FYI @nwmac @gaktive

@aalves08 aalves08 added [zube]: QA Review and removed [zube]: To Test QA/manual-test Indicates issue requires manually testing automation-candidate Candidate for being covered by automated tests labels Apr 2, 2024
@izaac
Copy link
Contributor

izaac commented Apr 3, 2024

The component unit test covers this and covers this scenario properly cc @yonasberhe23

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