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(core): fix Avatar Group's popover control styles & enhance a11y #5628

Merged
merged 5 commits into from
Jun 17, 2021
Merged

fix(core): fix Avatar Group's popover control styles & enhance a11y #5628

merged 5 commits into from
Jun 17, 2021

Conversation

artolshansky
Copy link
Contributor

@artolshansky artolshansky commented Jun 8, 2021 •
edited
Loading

BREAKING CHANGE:
added AvatarGroupPopoverControlDirective (fd-avatar-group-popover-control) needed to bind the corresponding CSS class and attributes to group type overflow popover control.

Please provide a link to the associated issue.

Closes #5575

Please provide a brief summary of this pull request.

  • a11y issues in examples
  • new directive for styling group type popover control

Please check whether the PR fulfills the following requirements

Documentation checklist:

@artolshansky artolshansky added accessibility use this label for any issue or enhancement related to screenreader/keyboard/etc support core Core library specific issues ATL Accessibility Test Labs labels Jun 8, 2021
@artolshansky artolshansky added this to the Sprint 64 - Ariba milestone Jun 8, 2021
@artolshansky artolshansky requested review from a team June 8, 2021 07:52
@artolshansky artolshansky self-assigned this Jun 8, 2021
@netlify
Copy link

netlify bot commented Jun 8, 2021 •
edited
Loading

✔️ Deploy Preview for fundamental-ngx ready!

🔨 Explore the source changes: 65b000d

🔍 Inspect the deploy log: https://app.netlify.com/sites/fundamental-ngx/deploys/60ca06e11677c200080c3da9

😎 Browse the preview: https://deploy-preview-5628--fundamental-ngx.netlify.app

@kavya-b
Copy link
Contributor

kavya-b commented Jun 8, 2021

Not sure if this PR covers this, but on tabbing and entering the Individual Type avatar details, focus does not enter the popover and screen reader does not read the popover contents. Checked on chromevox. The focus enters popover through the back button in the Group Type example though.

kavya-b
kavya-b approved these changes Jun 9, 2021
View reviewed changes
Copy link
Contributor

@kavya-b kavya-b left a comment

Choose a reason for hiding this comment

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

minor comment regarding exporting. Otherwise LGTM.

InnaAtanasova
InnaAtanasova reviewed Jun 9, 2021
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.

not really clear from the title what this PR is fixing

@artolshansky artolshansky changed the title fix(core): styles of group popover control & a11y issues fix(core): Avatar Group's popover control styles & enhance a11y Jun 10, 2021
@artolshansky artolshansky changed the title fix(core): Avatar Group's popover control styles & enhance a11y fix(core): fix Avatar Group's popover control styles & enhance a11y Jun 10, 2021
@github-actions github-actions bot added the stale label Jun 14, 2021
@lgtm-com
Copy link

lgtm-com bot commented Jun 14, 2021

This pull request introduces 1 alert when merging 5fc7505 into 5501a94 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@SAP SAP deleted a comment from github-actions bot Jun 14, 2021
@artolshansky artolshansky added the ready for qa Testing should be started for the PR label Jun 15, 2021
@puru-hk
Copy link
Contributor

puru-hk commented Jun 16, 2021

Issue 4:
i can able to reproduce,

Issue 5
Fixed

Issue 6:
OB; The dialog details is not read.
EB: The dialog name followed by the details and group information 'Contact' should be read when phone number is accessed at one go.
image

@puru-hk puru-hk added rework required Validation failed and removed ready for qa Testing should be started for the PR labels Jun 16, 2021
@artolshansky artolshansky added ready for qa Testing should be started for the PR and removed rework required Validation failed labels Jun 16, 2021
@puru-hk puru-hk added QA Approved and removed ready for qa Testing should be started for the PR labels Jun 17, 2021
@artolshansky artolshansky merged commit 3d9def3 into main Jun 17, 2021
@artolshansky artolshansky deleted the fix/avatar-group-a11y branch June 17, 2021 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kavya-b kavya-b kavya-b approved these changes

@InnaAtanasova InnaAtanasova InnaAtanasova approved these changes

@puru-hk puru-hk puru-hk approved these changes

@platon-rov platon-rov platon-rov approved these changes

@Betrozov Betrozov Betrozov approved these changes

Assignees

@artolshansky artolshansky

Labels
accessibility use this label for any issue or enhancement related to screenreader/keyboard/etc support ATL Accessibility Test Labs core Core library specific issues QA Approved
Projects
None yet
Milestone
Sprint 64 - Ariba
Development

Successfully merging this pull request may close these issues.

Avatar Group-Core
6 participants
@artolshansky @kavya-b @puru-hk @Betrozov @platon-rov @InnaAtanasova