Skip to content

Ability to anchor modals relatively to its context rather than just the body. #105

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

Closed
wants to merge 2 commits into from

Conversation

addnab
Copy link

@addnab addnab commented Nov 22, 2015

This can be done just by setting the prop relative = true.

Here are the possible use cases:

  • When you want a modal to appear on a particular part of the page.
  • Multiple modals to appear on different parts of the page.

I don't think the current release solves these use cases already. Please correct me if I'm wrong.

@evoyy
Copy link
Contributor

evoyy commented Nov 26, 2015

I believe this should already be possible. If you look in the README it says:

/*
By default the modal is anchored to document.body. All of the following overrides are available.

* element
Modal.setAppElement(appElement);

* query selector - uses the first element found if you pass in a class.
Modal.setAppElement('#your-app-element');
*/

However, I noticed that this is not working correctly. I have opened this issue #107

@addnab
Copy link
Author

addnab commented Nov 26, 2015

@evoyy I did notice the note on the README. But I found all it did was set/unset the aria-hidden attribute on whatever AppElement you provided it with.

@evoyy
Copy link
Contributor

evoyy commented Nov 26, 2015

I noticed that also. I think we will have to wait for clarification from the maintainers. I have just submitted a PR #108 which makes the modal do what I had expected it to do.

@addnab
Copy link
Author

addnab commented Nov 26, 2015

I don't find the approach of having to set the element you want to anchor the modal to as a prop very elegant. It would feel nicer to just plug the Modal Component into whichever element you'd like a modal to be over.

@evoyy
Copy link
Contributor

evoyy commented Nov 26, 2015

I agree. I removed that suggestion from my PR. I will be happy to go with your solution.

@claydiffrient
Copy link
Contributor

I'm not sure that this particular request fits in line with what this component is trying to accomplish. This component is intended to be a very accessible modal. I'm having a hard time figuring out how the use cases you mentioned work from a UX perspective.

Multiple modals to appear on different parts of the page.

Generally when a modal is open, the entire experience focuses on that modal being open, that's why everything behind it fades out. That essentially means that only one modal should ever be open at a time.

When you want a modal to appear on a particular part of the page.

As I mentioned earlier, a modal takes focus of the entire user experience and should essentially take up the entire page, rather than a particular section of it.

Thanks for the PR, but I'm going to close it because I don't think its that good of an idea at this point. If there is significant interest in the future, perhaps we will reconsider.

@addnab
Copy link
Author

addnab commented Mar 28, 2016

I understand. Thanks.

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.

3 participants