Modal component to update props when it receive new ones and fix erro… #507
Conversation
…r with react-modal
|
Travis automatic deployment: https://release-507-pr-onfido-sdk-ui-onfido.surge.sh |
|
Travis automatic deployment: https://staging-507-pr-onfido-sdk-ui-onfido.surge.sh |
|
Travis automatic deployment: https://507-pr-onfido-sdk-ui-onfido.surge.sh |
|
Travis automatic deployment: https://staging-507-pr-onfido-sdk-ui-onfido.surge.sh |
|
Travis automatic deployment: https://507-pr-onfido-sdk-ui-onfido.surge.sh |
|
Travis automatic deployment: https://release-507-pr-onfido-sdk-ui-onfido.surge.sh |
MIGRATION.md
Outdated
| // callback for when everything is complete | ||
| console.log("everything is complete") | ||
| }, | ||
| steps: [ |
There was a problem hiding this comment.
Minor: to keep the migration more focused you can remove the steps and let it default
README.md
Outdated
| // callback for when everything is complete | ||
| console.log("everything is complete") | ||
| }, | ||
| steps: [ |
There was a problem hiding this comment.
you can drop steps from here too.
on a side note: I'm a bit hesitant with adding long examples to side features. My fear is the readme can grow quite large
src/components/Modal/index.js
Outdated
| portalClassName={style.portal} | ||
| overlayClassName={style.overlay} | ||
| bodyClassName={style.modalBody} | ||
| className={style.inner} | ||
| shouldCloseOnOverlayClick={true} | ||
| closeTimeoutMS={MODAL_ANIMATION_DURATION} | ||
| ariaHideApp={false} |
There was a problem hiding this comment.
It prevents react-modal from throwing You must set an element with Modal.setAppElement(el) to make this accessible. Thinking about it, this is probably not the best way of fixing the error because this will make the content behind the modal readable by screen readers. There is another fix and it involves passing appElement={document.body} or appElement={document.getElementById('onfido-mount')}. I think this is a better solution because it doesn't break screen readers. See discussion here reactjs/react-modal#576
|
Travis automatic deployment: https://staging-507-pr-onfido-sdk-ui-onfido.surge.sh |
|
Travis automatic deployment: https://507-pr-onfido-sdk-ui-onfido.surge.sh |
|
Travis automatic deployment: https://release-507-pr-onfido-sdk-ui-onfido.surge.sh |
|
Travis automatic deployment: https://staging-507-pr-onfido-sdk-ui-onfido.surge.sh |
|
Travis automatic deployment: https://release-507-pr-onfido-sdk-ui-onfido.surge.sh |
|
Travis automatic deployment: https://507-pr-onfido-sdk-ui-onfido.surge.sh |
Problem
useModal: true,react-modalreturnsUncaught Error: react-modal: You must set an element withModal.setAppElement(el)to make this accessibleYou can see the error here https://jsfiddle.net/4xqtt6fL/1698/
isModalOpenoption was not properly received by theModalcomponent and this was causing issues when trying to close the modal using the close button.Solution
appElement={document.body}to thereact-modalcomponent so it knows which element should be hidden from screen readers when the modal is open.componentWillReceiveProps(nextProps) {...}to update the state of the ModalChecklist
put
n/aif item is not relevant to PR changes