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(Modal): get modal container from state to set/unset aria-hidden #2406

Merged
merged 1 commit into from Jul 2, 2019

Conversation

@boaz0
Copy link
Member

boaz0 commented Jun 30, 2019

What:
fixes #2405

When displaying a modal both the children of the body and the modal container had aria-hidden=true. The problem was that the container that was compared with the children is not stored in this.container but in the state.

This PR fixes this problem.

//cc @jgiardino

Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Jun 30, 2019

PatternFly-React preview: https://patternfly-react-pr-2406.surge.sh

@dlabaj
dlabaj approved these changes Jul 1, 2019
Copy link
Contributor

dlabaj left a comment

@jgiardino Can you make sure this resolves any accessibility concerns. Thanks for the fix @boaz0

@dlabaj dlabaj self-assigned this Jul 1, 2019
@dlabaj dlabaj added the bug 🐛 label Jul 1, 2019
@jenny-s51 jenny-s51 self-requested a review Jul 2, 2019
Copy link
Contributor

jenny-s51 left a comment

Currently working on the same issue in AboutModal and this change looks good to me.

@tlabaj
tlabaj approved these changes Jul 2, 2019
Copy link
Contributor

tlabaj left a comment

LGTM

@tlabaj tlabaj merged commit dd49c7e into patternfly:master Jul 2, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
@jgiardino

This comment has been minimized.

Copy link
Collaborator

jgiardino commented Jul 2, 2019

Thanks for working on this, @boaz0!!

bitmoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.