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: add missing role button attribute to the button links #19535

Merged
merged 15 commits into from
Nov 25, 2024
Merged

fix: add missing role button attribute to the button links #19535

merged 15 commits into from
Nov 25, 2024

Conversation

uroslates
Copy link
Contributor

@uroslates uroslates requested a review from a team as a code owner November 13, 2024 10:30
@github-actions github-actions bot marked this pull request as draft November 13, 2024 10:30
@uroslates uroslates marked this pull request as ready for review November 13, 2024 11:21
@uroslates uroslates requested review from a team as code owners November 13, 2024 11:21
@github-actions GitHub Actions
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 November 13, 2024 11:22
@uroslates uroslates marked this pull request as ready for review November 13, 2024 11:22
@cypress Cypress
Copy link

cypress bot commented Nov 13, 2024 •
edited
Loading

spartacus    Run #45945

Run Properties:  status check passed Passed #45945  •  git commit f345502a75 ℹ️: Merge c59023c0cd9f227c4100e632a14a02fbefb8111c into 49ab000d7b53ed24f64a492bad95...
Project spartacus
Branch Review feature/CXSPA-8806
Run status status check passed Passed #45945
Run duration 14m 49s
Commit git commit f345502a75 ℹ️: Merge c59023c0cd9f227c4100e632a14a02fbefb8111c into 49ab000d7b53ed24f64a492bad95...
Committer Uros Lates
View all properties for this run ↗︎
Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 4
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
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

Pio-Bar
Pio-Bar previously approved these changes Nov 13, 2024
View reviewed changes
Zeyber
Zeyber previously approved these changes Nov 14, 2024
View reviewed changes
@developpeurweb
Copy link
Contributor

@uroslates as per requirement in the ticket, it must be possible to interact with converted elements with both Spacebar and Enter keys.
Currently they can only be activated using Enter as regular links. Can you please make sure all converted elements can also be activated using Spacebar.
Please ping me if you have any questions.
Thanks!

@uroslates
Copy link
Contributor Author

@uroslates as per requirement in the ticket, it must be possible to interact with converted elements with both Spacebar and Enter keys. Currently they can only be activated using Enter as regular links. Can you please make sure all converted elements can also be activated using Spacebar. Please ping me if you have any questions. Thanks!

Hi @developpeurweb, thanks for the message. Maybe I'm missing something but I was able to interact with links by using both Enter/Space buttons in Win/JAWS.

@github-actions github-actions bot marked this pull request as draft November 19, 2024 09:37
@uroslates uroslates dismissed stale reviews from Zeyber and Pio-Bar via 141f8af November 20, 2024 10:12
@uroslates uroslates marked this pull request as ready for review November 21, 2024 15:41
@uroslates
Copy link
Contributor Author

uroslates commented Nov 21, 2024 •
edited
Loading

@Zeyber I have added new Directive here for addressing spacebar/enter keys as specified by Miguel, but if using simple buttons instead of links is acceptable solution I can reiterate over this again. Please feel free to let me know what U think. Thanks!

Zeyber
Zeyber previously approved these changes Nov 22, 2024
View reviewed changes
@developpeurweb
Copy link
Contributor

@uroslates the changes work well. One last thing though, can you please include the order cancellation buttons rework (screenshot below) on your PR. This was on @petarmarkov9449 end, but an auditor suddenly closed his ticket, so let's leave that ticket closed and introduce the changes in your PR. Thanks!

link to btn 1
link to btn 2
link to btn 3

@github-actions github-actions bot marked this pull request as draft November 25, 2024 10:02
@uroslates
Copy link
Contributor Author

co-author @petarmarkov9449

@uroslates
Copy link
Contributor Author

@uroslates the changes work well. One last thing though, can you please include the order cancellation buttons rework (screenshot below) on your PR. This was on @petarmarkov9449 end, but an auditor suddenly closed his ticket, so let's leave that ticket closed and introduce the changes in your PR. Thanks!

@developpeurweb We have together with @petarmarkov9449 incorporated all of his changes (from his PR) into this PR. Hopefully it'll be all OK now.. Cheers!

@uroslates uroslates marked this pull request as ready for review November 25, 2024 11:47
@uroslates uroslates requested a review from a team as a code owner November 25, 2024 11:47
@github-actions github-actions bot marked this pull request as draft November 25, 2024 12:36
@cypress Cypress
Copy link

cypress bot commented Nov 25, 2024 •
edited
Loading

spartacus    Run #45946

Run Properties:  status check passed Passed #45946  •  git commit c59023c0cd: Merge branch 'develop' into feature/CXSPA-8806
Project spartacus
Branch Review feature/CXSPA-8806
Run status status check passed Passed #45946
Run duration 53m 33s
Commit git commit c59023c0cd: Merge branch 'develop' into feature/CXSPA-8806
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 22
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 811
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

@uroslates uroslates marked this pull request as ready for review November 25, 2024 13:01
@uroslates uroslates merged commit fc5e624 into develop Nov 25, 2024
49 checks passed
@uroslates uroslates deleted the feature/CXSPA-8806 branch November 25, 2024 15:29
uroslates added a commit that referenced this pull request Nov 25, 2024
@uroslates @ulates-sap
fix: add missing role button attribute to the button links (#19535)
45d7ecc 
Co-authored-by: Peter Markov <petarmarkov9449>
uroslates added a commit that referenced this pull request Nov 25, 2024
@uroslates
fix: add missing role button attribute to the button links (#19535) (#…
4ba029d 
…19620)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@developpeurweb developpeurweb developpeurweb approved these changes

@Pio-Bar Pio-Bar Pio-Bar left review comments

@Zeyber Zeyber Zeyber left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@uroslates @developpeurweb @Zeyber @Pio-Bar @ulates-sap