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: Migrate nested list icon due to 0.12.0 styles #3390

Merged
merged 2 commits into from
Sep 25, 2020
Merged

fix: Migrate nested list icon due to 0.12.0 styles #3390

merged 2 commits into from
Sep 25, 2020

Conversation

JKMarkowski
Copy link
Contributor

@JKMarkowski JKMarkowski commented Sep 24, 2020 •
edited

Please provide a link to the associated issue.

part of #3370

Please provide a brief summary of this pull request.

There are only A11y changes included.
From:

<span fd-nested-list-icon [glyph]="'menu'"></span>

To:

<i fd-nested-list-icon glyph="menu"></i>

Please check whether the PR fulfills the following requirements

@JKMarkowski JKMarkowski requested a review from a team September 24, 2020 14:48
@JKMarkowski JKMarkowski added this to In progress in Development via automation Sep 24, 2020
@JKMarkowski JKMarkowski added this to the Sprint 46 - Montreal milestone Sep 24, 2020
@netlify
Copy link

netlify bot commented Sep 24, 2020 •
edited

Deploy preview for fundamental-ngx ready!

Built with commit d5574b1

https://deploy-preview-3390--fundamental-ngx.netlify.app

@droshev droshev force-pushed the fix/migrate-nested-list-0.12.0 branch from a66224f to 08491ad Compare September 25, 2020 01:02
salarenko
salarenko reviewed Sep 25, 2020
View reviewed changes
Copy link
Contributor

@salarenko salarenko left a comment

Choose a reason for hiding this comment

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

LGTM, but use static binding then possible and I think we should move role="presentation" to fd-nested-list-icon and set it by default.

@salarenko salarenko added core Core library specific issues icons labels Sep 25, 2020
@droshev droshev merged commit cd3e708 into master Sep 25, 2020
Development automation moved this from In progress to Done Sep 25, 2020
@droshev droshev deleted the fix/migrate-nested-list-0.12.0 branch September 25, 2020 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@droshev droshev droshev approved these changes

@InnaAtanasova InnaAtanasova InnaAtanasova approved these changes

@salarenko salarenko Awaiting requested review from salarenko

Assignees

@JKMarkowski JKMarkowski

Labels
core Core library specific issues
Projects
None yet
Milestone
Sprint 46 - Montreal
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@JKMarkowski @droshev @salarenko @InnaAtanasova