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

Side navigation status labels and icons #2928

Merged
merged 4 commits into from Mar 17, 2020

Conversation

bartaz
Copy link
Contributor

@bartaz bartaz commented Mar 16, 2020

Done

Add right side status icons/labels to side navigation.

Part of #2862

QA

  • ./run or demo
  • Check if labels and icons are displayed correctly on different levels of navigation

Screenshots

Screenshot 2020-03-16 at 16 22 57

@bartaz bartaz added the Feature 🎁 New feature or request label Mar 16, 2020
@webteam-app
Copy link

@bartaz bartaz changed the title WIP: Side navigation status Side navigation status labels and icons Mar 16, 2020
@bartaz bartaz marked this pull request as ready for review March 16, 2020 15:23
@lyubomir-popov
Copy link
Contributor

@bartaz I think the margin-right on the icon should be 2x bigger. This is so make it consistent with rules we're creating for icons elsewhere: .5rem offset between icon and the text it belongs to, 1rem between icon and unrelated text.

@lyubomir-popov
Copy link
Contributor

@bartaz We already have an example for truncation in the utilities section, but wondering if it would be helpful to have one here as well. The gotcha there is, the truncated text needs to be wrapped in a span with class"u-truncate", rather than adding that class on the <li>. Only if you think it is a good idea, not insisting.
image

@bartaz
Copy link
Contributor Author

bartaz commented Mar 17, 2020

@lyubomir-popov

I updated the spacing on the right of icons.

As for truncating - I have a separate task #2865 to make sure we have all cases in examples, so will make sure I add it there.

Copy link
Contributor

@lyubomir-popov lyubomir-popov left a comment

Choose a reason for hiding this comment

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

LGTM, just tiny var rename.

scss/_patterns_side-navigation.scss Outdated Show resolved Hide resolved
@bartaz bartaz merged commit f059418 into canonical:master Mar 17, 2020
@bartaz bartaz deleted the side-navigation-status branch March 17, 2020 10:36
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.

None yet

3 participants