Skip to content

Fix DAP issues for Modal #161

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

Merged
merged 4 commits into from
Oct 16, 2018

Conversation

aradhikaIbm
Copy link
Contributor

@aradhikaIbm aradhikaIbm commented Oct 8, 2018

Changes made:

  1. change role from main to dialog on div
  2. add label for dialog role
  3. remove banner and content-info per html 5 usage
  4. Add any to service method for compiling. To resolve this: error TS1122: A tuple type element list cannot be empty.

Closes IBM/carbon-components-angular #114
fixes #158

Resolves the DAP violations for the modal

Changelog

Changed

  • Changes to the various attributes on the modal to comply with accessibility

change role from main to dialog on div
add label for dialog role
remove banner and content-info per html 5 usage
any to service method for compiling
cal-smith
cal-smith previously approved these changes Oct 9, 2018
Copy link
Contributor

@cal-smith cal-smith left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@aradhikaIbm
Copy link
Contributor Author

Can we close on this change ?

@cal-smith
Copy link
Contributor

Waiting for another review ... @zvonimirfras ?

@@ -87,7 +87,7 @@ export class ModalService {
* @returns {ComponentRef<any>}
* @memberof ModalService
*/
show(data: AlertModalData) {
Copy link
Member

Choose a reason for hiding this comment

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

This should stay indented. :)

@zvonimirfras zvonimirfras changed the title Issue #114 - Fix DAP issues for Modal Fix DAP issues for Modal Oct 15, 2018
@zvonimirfras zvonimirfras merged commit 40e5148 into carbon-design-system:master Oct 16, 2018
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.

Unable to run storybook due to Typescript error in Terminal
3 participants