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

[JS][Renderer][Accessibility] Action.OpenUrl rendered buttons should have the "link" aria role #3914

Merged
merged 2 commits into from
Apr 29, 2020

Conversation

paulcam206
Copy link
Member

@paulcam206 paulcam206 commented Apr 27, 2020

Details

Currently all actions get rendered as <button> elements with role "button". When a button opens a link, as is the case for Action.OpenUrl, it should have the "link" role instead.

Important
Just a note on the weirdness of this PR. I used a new tool to create this PR, which converted the issue into a PR rather than creating a new PR. I won't use it again, but it's why this looks weird. 👍

Description

Simple fix to have OpenUrlAction objects set role="link".

How verified

  • Accessibility Insights
  • F12 tools
  • Tests (adding shortly)
Microsoft Reviewers: Open in CodeFlow

@dclaux dclaux added the Bug label Apr 9, 2020
@ghost ghost added the Triage-Needed label Apr 9, 2020
@ghost ghost added this to Needs triage in Bug Triage Apr 9, 2020
@dclaux dclaux self-assigned this Apr 9, 2020
@shalinijoshi19 shalinijoshi19 added AdaptiveCards v1.2.9 Platform-JavaScript Bugs or features related to the JavaScript renderer labels Apr 9, 2020
@shalinijoshi19 shalinijoshi19 moved this from Needs triage to Approved in Bug Triage Apr 9, 2020
@ghost
Copy link

ghost commented Apr 9, 2020

We have acknowledged this issue report. Please continue to follow the issue for updates/progress/questions. @matthidinger / @dclaux / @RebeccaAnne / @paulcam206 / @nesalang / @almedina-ms FYI.

@shalinijoshi19 shalinijoshi19 assigned paulcam206 and unassigned dclaux Apr 27, 2020
@shalinijoshi19
Copy link
Member

shalinijoshi19 commented Apr 27, 2020

@paulcam206 wanna take a stab at this unless @dclaux already has been working on it? This is a partner (Dynamics) accessibility blocker

@ghost
Copy link

ghost commented Apr 27, 2020

Thanks for helping make the AdaptiveCards JS renderer + tooling better. As additional verification, once the JS build succeeds, please go to the test site to test out your website/designer changes

@shalinijoshi19
Copy link
Member

shalinijoshi19 commented Apr 28, 2020

@paulcam206 wasnt there an issue backing this PR? I could have sworn this was an issue not a PR and it's gotten pretty confusing with the triage etc.. Whats going on here?

@shalinijoshi19
Copy link
Member

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@paulcam206
Copy link
Member

@paulcam206 wasnt there an issue backing this PR? I could have sworn this was an issue not a PR and it's gotten pretty confusing with the triage etc.. Whats going on here?

yep - as I noted in the description, I used a new tool to generate the PR not knowing that it would just convert the issue into a PR rather than create a new PR 😬

@paulcam206
Copy link
Member

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@paulcam206
Copy link
Member

@dclaux approved over Teams

@paulcam206 paulcam206 merged commit 3dfc793 into master Apr 29, 2020
Bug Triage automation moved this from Approved to Closed Apr 29, 2020
@paulcam206 paulcam206 deleted the paulcam/ts-button-link-aria branch April 29, 2020 21:26
jwoo-msft added a commit that referenced this pull request May 6, 2020
* updated visualizer for template support

* [UWP] Remove weak reference to ellipse object in image renderer (#3739)

* [UWP] Remove weak reference to ellipse object in image renderer

Fixes #3720

* Remove weak reference to ellipse
* Fix ImageBrush leak
* Add person style to Image.BackgroundColor sample to catch regressions
* Update samples

* Update weak ref comment

Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>

* updated visualizer for template support

* Added Templat support to visualizer

* Addes Samples Scenario for Template

* [.NET] Stub out _CheckForGenerateAppxPackageOnBuild target (#3966)

This is a workaround for a regression in VS2019 16.5.0. A fix has been checked in, but won't ship until 16.7.1 Preview 1, so this workaround will have to do for now.

* Fixed inputs not passed while used in ActionSet (#3962)

* [JS] Render Action.OpenUrl with role=link (#3914)

Fixes #3914

* Added Template Element Cards

* updated for review comments

* fixed ci build error and added custom function unit test

* updated visualizer for template support

* Added Templat support to visualizer

* Addes Samples Scenario for Template

* Added Template Element Cards

* updated sample files for Tempalte Elements

* Updated sample cards

* Updated samples with dateFormat

Co-authored-by: Paul Campbell <paulcam@microsoft.com>
Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>
jwoo-msft added a commit that referenced this pull request May 11, 2020
* Added a nuget.config for daily build with AEL MyGet

* added suffix variable for template ci build

* Added VersionPrefix

* updated version prefix

* Updated version

* Updated WPF visualizer with template (#3958)

* updated visualizer for template support

* [UWP] Remove weak reference to ellipse object in image renderer (#3739)

* [UWP] Remove weak reference to ellipse object in image renderer

Fixes #3720

* Remove weak reference to ellipse
* Fix ImageBrush leak
* Add person style to Image.BackgroundColor sample to catch regressions
* Update samples

* Update weak ref comment

Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>

* updated visualizer for template support

* Added Templat support to visualizer

* Addes Samples Scenario for Template

* [.NET] Stub out _CheckForGenerateAppxPackageOnBuild target (#3966)

This is a workaround for a regression in VS2019 16.5.0. A fix has been checked in, but won't ship until 16.7.1 Preview 1, so this workaround will have to do for now.

* Fixed inputs not passed while used in ActionSet (#3962)

* [JS] Render Action.OpenUrl with role=link (#3914)

Fixes #3914

* Added Template Element Cards

* updated for review comments

* fixed ci build error and added custom function unit test

* updated visualizer for template support

* Added Templat support to visualizer

* Addes Samples Scenario for Template

* Added Template Element Cards

* updated sample files for Tempalte Elements

* Updated sample cards

* Updated samples with dateFormat

Co-authored-by: Paul Campbell <paulcam@microsoft.com>
Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>

* Updated surface API

* Added a nuget.config for daily build with AEL MyGet

* added suffix variable for template ci build

* Added VersionPrefix

* updated version prefix

* Updated version

* Updated to RC4 of AEL Updated Samples

* Added a nuget.config for daily build with AEL MyGet

* added suffix variable for template ci build

* Added VersionPrefix

* updated version prefix

* Updated version

* added suffix variable for template ci build

* Added VersionPrefix

Co-authored-by: Paul Campbell <paulcam@microsoft.com>
Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>
jwoo-msft added a commit that referenced this pull request May 11, 2020
* migrated codes to dotnet source

* updated surface api

* added parser debugging tool & update names

* Added comments and cleaned up the codes

* Refactored code, added more comments and updated README

* Added API marked down and added more comments

* formatting fixes

* updated DataContext and addressed CR comments

* added more comments

* Updated grammar

* fixed ci build error and added custom function unit test

* Updated WPF visualizer with template (#3958)

* updated visualizer for template support

* [UWP] Remove weak reference to ellipse object in image renderer (#3739)

* [UWP] Remove weak reference to ellipse object in image renderer

Fixes #3720

* Remove weak reference to ellipse
* Fix ImageBrush leak
* Add person style to Image.BackgroundColor sample to catch regressions
* Update samples

* Update weak ref comment

Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>

* updated visualizer for template support

* Added Templat support to visualizer

* Addes Samples Scenario for Template

* [.NET] Stub out _CheckForGenerateAppxPackageOnBuild target (#3966)

This is a workaround for a regression in VS2019 16.5.0. A fix has been checked in, but won't ship until 16.7.1 Preview 1, so this workaround will have to do for now.

* Fixed inputs not passed while used in ActionSet (#3962)

* [JS] Render Action.OpenUrl with role=link (#3914)

Fixes #3914

* Added Template Element Cards

* updated for review comments

* fixed ci build error and added custom function unit test

* updated visualizer for template support

* Added Templat support to visualizer

* Addes Samples Scenario for Template

* Added Template Element Cards

* updated sample files for Tempalte Elements

* Updated sample cards

* Updated samples with dateFormat

Co-authored-by: Paul Campbell <paulcam@microsoft.com>
Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>

* Updated surface API

* Updated to RC4 of AEL Updated Samples

* .Net Template Internal Feed Changes (#3986)

* Added a nuget.config for daily build with AEL MyGet

* added suffix variable for template ci build

* Added VersionPrefix

* updated version prefix

* Updated version

* Updated WPF visualizer with template (#3958)

* updated visualizer for template support

* [UWP] Remove weak reference to ellipse object in image renderer (#3739)

* [UWP] Remove weak reference to ellipse object in image renderer

Fixes #3720

* Remove weak reference to ellipse
* Fix ImageBrush leak
* Add person style to Image.BackgroundColor sample to catch regressions
* Update samples

* Update weak ref comment

Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>

* updated visualizer for template support

* Added Templat support to visualizer

* Addes Samples Scenario for Template

* [.NET] Stub out _CheckForGenerateAppxPackageOnBuild target (#3966)

This is a workaround for a regression in VS2019 16.5.0. A fix has been checked in, but won't ship until 16.7.1 Preview 1, so this workaround will have to do for now.

* Fixed inputs not passed while used in ActionSet (#3962)

* [JS] Render Action.OpenUrl with role=link (#3914)

Fixes #3914

* Added Template Element Cards

* updated for review comments

* fixed ci build error and added custom function unit test

* updated visualizer for template support

* Added Templat support to visualizer

* Addes Samples Scenario for Template

* Added Template Element Cards

* updated sample files for Tempalte Elements

* Updated sample cards

* Updated samples with dateFormat

Co-authored-by: Paul Campbell <paulcam@microsoft.com>
Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>

* Updated surface API

* Added a nuget.config for daily build with AEL MyGet

* added suffix variable for template ci build

* Added VersionPrefix

* updated version prefix

* Updated version

* Updated to RC4 of AEL Updated Samples

* Added a nuget.config for daily build with AEL MyGet

* added suffix variable for template ci build

* Added VersionPrefix

* updated version prefix

* Updated version

* added suffix variable for template ci build

* Added VersionPrefix

Co-authored-by: Paul Campbell <paulcam@microsoft.com>
Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>

Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>
Co-authored-by: Paul Campbell <paulcam@microsoft.com>
@shalinijoshi19 shalinijoshi19 added the Area-Accessibility Bugs around feature accessibility label May 20, 2020
@shalinijoshi19
Copy link
Member

@paulcam206 did this make it in to release/1.2 for 1.2.9 ? (sorry it missed my radar since it was converted to a PR and I'd been tracking issues)

paulcam206 added a commit that referenced this pull request May 28, 2020
paulcam206 added a commit that referenced this pull request May 29, 2020
Fixes #3914 in release/1.2 (this is also the PR)
Fixes #2240 in release/1.2 (PR #4059)
paulcam206 added a commit that referenced this pull request May 29, 2020
Fixes #3914 in release/1.2 (this is also the PR)
Fixes #2240 in release/1.2 (PR #4059)
@ghost
Copy link

ghost commented Jun 1, 2020

🎉AdaptiveCards@v1.2.9 has been released which fixes this issue.:tada:

Handy links:

@shalinijoshi19 shalinijoshi19 added this to the 20.05 milestone Jun 8, 2020
@InduPriya1805
Copy link

@shalinijoshi19 Is this bug got fixed and changes are available in the latest release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Accessibility Bugs around feature accessibility Area-Renderers Bug Platform-JavaScript Bugs or features related to the JavaScript renderer
Projects
No open projects
Bug Triage
  
Closed
Development

Successfully merging this pull request may close these issues.

None yet

4 participants