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

fix: (CXSPA-9075) fix footer region group labels arialLabel issue. #19875

Merged
merged 7 commits into from
Jan 20, 2025

Conversation

uroslates
Copy link
Contributor

@uroslates uroslates commented Jan 16, 2025

@uroslates uroslates requested a review from a team as a code owner January 16, 2025 12:32
@github-actions github-actions bot marked this pull request as draft January 16, 2025 12:33
@uroslates uroslates marked this pull request as ready for review January 16, 2025 12:43
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build. 
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@github-actions github-actions bot marked this pull request as draft January 16, 2025 12:43
@uroslates uroslates marked this pull request as ready for review January 16, 2025 12:48
Copy link

cypress bot commented Jan 16, 2025

spartacus    Run #46743

Run Properties:  status check passed Passed #46743  •  git commit 07d28c8b25 ℹ️: Merge 0f2968b6765b524f274ae4d4f6b92ce4f7bf8aab into ccebeb7d891a01a80f5a50abb44b...
Project spartacus
Branch Review feature/CXSPA-9075
Run status status check passed Passed #46743
Run duration 04m 11s
Commit git commit 07d28c8b25 ℹ️: Merge 0f2968b6765b524f274ae4d4f6b92ce4f7bf8aab into ccebeb7d891a01a80f5a50abb44b...
Committer Uros Lates
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 3
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 125
View all changes introduced in this branch ↗︎

@@ -153,6 +153,8 @@
[attr.depth]="getTotalDepth(node)"
[attr.wrap-after]="node.children.length > wrapAfter ? wrapAfter : null"
[attr.columns]="getColumnCount(node.children.length)"
role="group"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should keep the role unchanged. This is a list with listitems inside, I am not sure why we would be grouping them all together instead. The aria-label will still be read and this way we are keeping the semantic role. This would affect all implementations of the navigation-ui not just the footer, we need to keep this in mind while testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed!

@github-actions github-actions bot marked this pull request as draft January 20, 2025 09:16
@uroslates uroslates marked this pull request as ready for review January 20, 2025 09:16
@github-actions github-actions bot marked this pull request as draft January 20, 2025 09:38
@uroslates uroslates requested a review from Pio-Bar January 20, 2025 09:39
@uroslates uroslates marked this pull request as ready for review January 20, 2025 09:39
@github-actions github-actions bot marked this pull request as draft January 20, 2025 11:40
@uroslates uroslates marked this pull request as ready for review January 20, 2025 11:41
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build. 
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@github-actions github-actions bot marked this pull request as draft January 20, 2025 11:56
@uroslates uroslates marked this pull request as ready for review January 20, 2025 12:00
@uroslates uroslates merged commit cef9700 into develop Jan 20, 2025
28 checks passed
@uroslates uroslates deleted the feature/CXSPA-9075 branch January 20, 2025 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants