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

Setting aria-live to shadowRoot element instead to the slot. #3389

Open
wants to merge 4 commits into
base: stage
Choose a base branch
from

Conversation

seanchoi-dev
Copy link
Contributor

@seanchoi-dev seanchoi-dev commented Dec 17, 2024

@seanchoi-dev seanchoi-dev requested a review from a team as a code owner December 17, 2024 21:55
Copy link
Contributor

aem-code-sync bot commented Dec 17, 2024

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

Copy link
Contributor

aem-code-sync bot commented Dec 17, 2024

Page Scores Audits Google
📱 /?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

libs/deps/mas/commerce.js Outdated Show resolved Hide resolved
libs/deps/mas/mas.js Outdated Show resolved Hide resolved
Copy link

codecov bot commented Dec 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.48%. Comparing base (d543279) to head (f08e770).

Additional details and impacted files
@@           Coverage Diff           @@
##            stage    #3389   +/-   ##
=======================================
  Coverage   96.48%   96.48%           
=======================================
  Files         254      254           
  Lines       59031    59029    -2     
=======================================
- Hits        56954    56953    -1     
+ Misses       2077     2076    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@seanchoi-dev seanchoi-dev added bug Something isn't working run-nala Run Nala Test Automation against PR needs-verification PR requires E2E testing by a reviewer labels Dec 17, 2024
Copy link
Contributor

@vhargrave vhargrave left a comment

Choose a reason for hiding this comment

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

I wonder if you could not add the aria-live attribute while directly adding that text field to your component, but doesn't matter too much
looks good!

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

eslint

⚠️ [eslint] reported by reviewdog 🐶
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.

https://github.com/adobecom/milo/blob/f08e770dca9129c70681385f3364da63d0b8ac7d/libs/deps/mas/merch-whats-included.js


⚠️ [eslint] reported by reviewdog 🐶
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.

https://github.com/adobecom/milo/blob/f08e770dca9129c70681385f3364da63d0b8ac7d/libs/features/mas/src/merch-card-collection.js

Copy link
Contributor

This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label.

@github-actions github-actions bot added the Stale label Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-verification PR requires E2E testing by a reviewer run-nala Run Nala Test Automation against PR Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants