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
Add side navigation component with default styling #2915
Conversation
a8dfd78
to
35747d3
Compare
@bartaz we've just had our weekly product design review which produced some interesting feedback (thanks @cristinadresch), which I've just discussed with @lyubomir-popov. Can we try updating the selected states please, to simplify things a bit:
Now that we can see it in action, we may need to tweak things like this to get it right, hope that's ok. This shows the difference, on the left is what we have now: Thanks. |
@spencerbygraves Sure, this will simplify the code as well and remove a need for confusing additional 'selected' state on non active items. |
@bartaz @anthonydillon @lyubomir-popov if you look at the charmhub side nav, you'll see functionality to toggle on/off filters for the main content. Should we add that to the Vanilla side nav at some point? https://charmhub-io-canonical-web-and-design-pr-5.run.demo.haus/ The side nav here will be updated with the Vanilla one. |
@spencerbygraves Couple of notes on that:
So, I think what we have in plan to implement should be allow for such functionality (with some small compromises). We could later iterate on improvements if needed. |
@lyubomir-popov I'll have a look - I just used existing mixins. I'll check if we can control it. |
@lyubomir-popov So it seems currently the only way to display outline above our absolutely positioned border is to hide the border on focus. I'm personally not sure if it's better - looks a bit like border is "disappearing" rather than getting covered by outline: But will leave that design decision to you - do we keep current default (dark border over outline) or should we implement workaround and hide border on focus? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Done
Fixes #2860
Fixes #2861
QA
./run
Screenshots