[chore] update readme and docs demos#1012
Conversation
| @@ -1,7 +0,0 @@ | |||
| # Using setAppElement | |||
There was a problem hiding this comment.
Maybe I'm missing historical context but I removed it because it feels more of a requirement than a suggestion since a large warning is logged to the console when it's omitted and since every other example already included it. But if it's needed, then sure, I'll add it back in just a few.
There was a problem hiding this comment.
Maybe the explaination is bad. setAppElement is a feature that is misused all the time.
Problem is that users always place the setAppElement together with the modal utilization,
which causes problems when testing and others (i'll try to reference them).
Suggestions are welcome.
There was a problem hiding this comment.
OK, added back in. No strong suggestions, I just assumed setAppElement wasn't as strongly recommended in the past or something and might not be needed today.
There was a problem hiding this comment.
Actually setAppElement must be used. Historically the default element that react-modal uses to add the aria-hidden attribute is wrong. By default it will use the document.body which will turn the whole page inaccessible.
This needs to come back. |
Closes #955.
Changes proposed:
setAppElementexample. A warning is now logged when the app element is not set so removed this one and instead setAppElement in all other demos.The demos themselves have remained largely unchanged apart from converting to functional components. They are still styled simply and do not set any extra props. All demos are stored in this CodePen collection.
Upgrade Path (for changed or removed APIs):
Acceptance Checklist:
CONTRIBUTING.md.