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(sidebar): wrap long menu item label in sidebar #2566

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ciiay
Copy link
Member

@ciiay ciiay commented Mar 12, 2025

Description

  • Modified Backstage SidebarItem styles to allow long menu item labels to wrap instead of being cut off.
  • Refactored code to simplify the codebase and improve sidebar menu display, especially on smaller screens.

Figma reference

Which issue(s) does this PR fix

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

Screenshot

image

Copy link

openshift-ci bot commented Mar 12, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign albarbaro 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:

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

Copy link
Contributor

@ciiay
Copy link
Member Author

ciiay commented Mar 13, 2025

/test e2e-tests

@ciiay ciiay force-pushed the rhidp-6010-long-menu-item-heading branch from 26acf42 to d397965 Compare March 13, 2025 15:49
Copy link
Contributor

@ciiay
Copy link
Member Author

ciiay commented Mar 13, 2025

/test e2e-tests

@ciiay
Copy link
Member Author

ciiay commented Mar 13, 2025

/test e2e-tests

1 similar comment
@ciiay
Copy link
Member Author

ciiay commented Mar 14, 2025

/test e2e-tests

@ciiay ciiay requested a review from ShiranHi March 17, 2025 14:16
@ShiranHi
Copy link

Thank you, @ciiay, for this change! I have two questions:

  1. Can we ensure the text at the second and third levels is aligned?
image01
  1. Is there a reason for the large padding below the second level?
image02

@ciiay
Copy link
Member Author

ciiay commented Mar 19, 2025

  1. Can we ensure the text at the second and third levels is aligned?

Fixed it, thanks.
image

  1. Is there a reason for the large padding below the second level?

It is not a gap, just end of the menu list. If I collapse the Administration dropdown it'll be empty space only. I moved "Docs" out of its parent menu item so you can see clearer how it is.
image

Copy link
Contributor

@ciiay
Copy link
Member Author

ciiay commented Mar 19, 2025

/test e2e-tests

1 similar comment
@ciiay
Copy link
Member Author

ciiay commented Mar 19, 2025

/test e2e-tests

@its-mitesh-kumar
Copy link
Member

its-mitesh-kumar commented Mar 21, 2025

Tested the PR on cluster .
A. What I observed Littlelonglearningpathweneedtomange is getting trimmed followed by dots but same words with space ie. Little long learning path we need to mange getting wrapped to multiple lines .
B. Do we support grand child in menu items , everything seems fine , only icon is not visible if we have some grand child of menu item .
Screenshot 2025-03-21 at 6 50 56 PM

  1. With below config
dynamicPlugins:
      frontend:
        default.main-menu-items:
          menuItems:
            default.home:
              title: Home
              icon: home
            default.list:
              title: References
              icon: bookmarks
            default.my-group:
              parent: default.list
            default.learning-path:
              title: asdfghjgfdsaDZFGHJBJHGZESRDTFYGUKHJDSADFZXGCHVNDHBGSZ≈CVBNMDFGHJKLHFGHJKL;KJHFGCUYYLIUYTKTDTGIKJH,GFDJFGDCGFILUYUYTRDXC,JHGBFDCJKHGCFGCJKKH
              parent: default.list
            default.homepage:
              title: HomePage 123
              icon: home
              parent: default.home
            default.create:
              title: Long Createing Title abcdefghjkllooiuysdfgvndfbhvjsbfs fcbdhjgvbhjkdsbchjdsbvjhdsb
              icon: add
              parent: default.home


Screenshot 2025-03-21 at 5 12 56 PM (2)

  1. with this config
dynamicPlugins:
      frontend:
        default.main-menu-items:
          menuItems:
            default.home:
              title: Home
              icon: home
            default.list:
              title: References
              icon: bookmarks
            default.my-group:
              parent: default.list
            default.learning-path:
              title: asdfghjgfdsaDZFGHJBJHGZESRDTFYGUKHJDSADFZXGCHVNDHBGSZ
              parent: default.my-group
            default.homepage:
              title: HomePage 123
              icon: home
              parent: default.home
            default.create:
              title: Long Creating Title Lets see 
              icon: add
              parent: default.homepage

Screenshot 2025-03-21 at 6 50 56 PM

Copy link
Member

@its-mitesh-kumar its-mitesh-kumar left a comment

Choose a reason for hiding this comment

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

Please see the comment #2566 (comment)

@ciiay
Copy link
Member Author

ciiay commented Mar 21, 2025

Hi @its-mitesh-kumar , thanks for the review.

A. What I observed Littlelonglearningpathweneedtomange is getting trimmed followed by dots but same words with space ie. Little long learning path we need to mange getting wrapped to multiple lines .

Fixed. Extreme long label or long words should be edge case that we don't need to worry about too much.
image

B. Do we support grand child in menu items , everything seems fine , only icon is not visible if we have some grand child of menu item .

This is designed like this, the third level menu items don't have an icon.

Hi @ShiranHi , I noticed that the previous Figma link you shared now always redirects to the cover link. I saw you update it in Bug ticket 188, and I attached that link to the PR description. It used to work before but now whatever I do it only goes to the cover page. Can you share the design page of sidebar menu here? And I wonder if there's a better way to navigate from the cover page to a specific page. I tried to figure it out from this RHDH core page, but I couldn't find it anywhere 😭

Copy link
Contributor

@ShiranHi
Copy link

Hi @ShiranHi , I noticed that the previous Figma link you shared now always redirects to the cover link. I saw you update it in Bug ticket 188, and I attached that link to the PR description. It used to work before but now whatever I do it only goes to the cover page. Can you share the design page of sidebar menu here? And I wonder if there's a better way to navigate from the cover page to a specific page. I tried to figure it out from this RHDH core page, but I couldn't find it anywhere 😭

Oh, sorry for that! Here’s the Figma link to the menu page.

You can try scrolling through the page list on the left in Figma to find all the components:

video.mov

@ciiay
Copy link
Member Author

ciiay commented Mar 21, 2025

Thank you @ShiranHi for the recording 💯 I must have over looked "Main navigation" from the Pages list. I was a bit confused by the wording "Pages" and thought sidebar didn't below to any pages. And tried searching with "Sidebar" but got no luck 😅

@ciiay ciiay requested a review from its-mitesh-kumar March 21, 2025 21:43
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.

3 participants