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.

Sign up for GitHub

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

Jump to bottom

fix: Side Navigation - Refactor icons usage #1656

Closed
wants to merge 1 commit into from
Closed

fix: Side Navigation - Refactor icons usage #1656

wants to merge 1 commit into from
+34 −32

Conversation

salarenko
Copy link
Contributor

@salarenko salarenko commented Sep 17, 2020 •
edited by droshev
Loading

Related Issue

Part of: #1603
and #1639

Description

This PR:

  • Styles icon element with icon mixins
  • Adds aria attributes

Before:

<span class="fd-nested-list__icon sap-icon--chain-link"></span>

After:

<span aria-hidden="true" class="fd-nested-list__icon sap-icon--chain-link"></span>

Please check whether the PR fulfills the following requirements

@salarenko salarenko added Enhancement New feature or request icons labels Sep 17, 2020
@salarenko salarenko added this to the Sprint 46 - Montreal milestone Sep 17, 2020
@salarenko salarenko requested a review from a team September 17, 2020 13:22
@salarenko salarenko self-assigned this Sep 17, 2020
@netlify
Copy link

netlify bot commented Sep 17, 2020

Deploy preview for fundamental-styles ready!

Built with commit 1bb7db7

https://deploy-preview-1656--fundamental-styles.netlify.app

InnaAtanasova
InnaAtanasova requested changes Sep 17, 2020
View reviewed changes
Copy link
Contributor

@InnaAtanasova InnaAtanasova left a comment

Choose a reason for hiding this comment

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

Screen Shot 2020-09-17 at 3 54 04 PM

src/_nested-list.scss
@include fd-reset();
@include fd-flex-center();
@include fd-icon-element-base() {
@include fd-reset();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure you need a reset here?

@droshev droshev added the 0.12.0 label Sep 18, 2020
@salarenko salarenko closed this Sep 21, 2020
@salarenko salarenko mentioned this pull request Sep 21, 2020
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@InnaAtanasova InnaAtanasova InnaAtanasova requested changes

@mikerodonnell89 mikerodonnell89 Awaiting requested review from mikerodonnell89

Assignees

@salarenko salarenko

Labels
Enhancement New feature or request
Projects
None yet
Milestone
Sprint 46 - Montreal
Development

Successfully merging this pull request may close these issues.
3 participants
@salarenko @InnaAtanasova @droshev