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

Core: Add prefix customization for navigation items #12521

Closed
wants to merge 1 commit into from

Conversation

tooppaaa
Copy link
Contributor

Issue: Lack of capability to add elements to

  • sidebar subheadings (most important one in my opinion)
  • sidebar items
    • folder
    • component
    • story

This PR aims as a proposal for this and basis of discussions

image

  • Should we keep double icons when prefix is set ?
  • Should it be called prefix ?

@tooppaaa tooppaaa added ui maintenance User-facing maintenance tasks labels Sep 19, 2020
@shilman shilman changed the title Core: Add capability to add prefix to manager items Core: Add prefix customization for navigation items Sep 20, 2020
@shilman shilman added feature request and removed maintenance User-facing maintenance tasks labels Sep 20, 2020
@shilman
Copy link
Member

shilman commented Sep 20, 2020

cc @domyen @ghengeveld

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Looks great & code is pretty simple. Great stuff @tooppaaa 💯

We're doing some work on the sidebar as part of the search stuff and I want to make sure this is compatible with that work before we merge it. My spider sense is tingling that we'll want to address this in a different way, but don't have a concrete suggestion yet.

@tooppaaa
Copy link
Contributor Author

Happy to wait / change implementation !
As I mentioned, this is simple "suggestion PR" where we can see whether we want this or not :)

@ghengeveld
Copy link
Member

ghengeveld commented Sep 20, 2020

I'm working on a revamp of the sidebar right now. Allowing customization is on the radar and the current proposal is to allow this by setting data attributes on relevant nodes for various things. That allows these elements to be targeted with CSS and read the data values from JS.

@tooppaaa would this work for you? Do you have other suggestions?

I'm not so keen on adding a prefix attribute because it's very use case specific. I'd rather have a more generic solution which plugins can hook into to add such a prefix that way.

@ghengeveld
Copy link
Member

@tooppaaa I can share the redesign strawman with me you if you'd like. Just send me your Google account address on which to invite you. I think you follow me on Twitter. Or drop a note in the chat bubble at https://www.chromatic.com/docs

@stale
Copy link

stale bot commented Oct 12, 2020

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Oct 12, 2020
@tooppaaa
Copy link
Contributor Author

@ghengeveld Should we drop that PR ?

@stale stale bot removed the inactive label Oct 15, 2020
@ndelangen
Copy link
Member

I think the idea on this PR is great, @tooppaaa I think it's worth continuing in concept, but perhaps it'd be easier to start anew rather then try and fix the merge-conflicts?

@tooppaaa
Copy link
Contributor Author

Reworked as #13121

@tooppaaa tooppaaa closed this Nov 15, 2020
@stof stof deleted the feature/prefixForManager branch May 25, 2022 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants