Skip to content

[fix] keep references of modals. #358

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 10, 2017
Merged

[fix] keep references of modals. #358

merged 1 commit into from
Apr 10, 2017

Conversation

diasbruno
Copy link
Collaborator

Keep a reference of all modals in order to manage
when to add/remove classes and aria from elements
correctly.

This solution is based on @MarkMurphy's idea #308, but it uses a simple list
to work on IE (don't really support Set).

Fixes #218 #231 #308.

Acceptance Checklist:

  • All commits have been squashed to one.
  • The commit message follows the guidelines in CONTRIBUTING.md.
  • Documentation (README.md) and examples have been updated as needed.
  • If this is a code change, a spec testing the functionality has been added.
  • If the commit message has [changed] or [removed], there is an upgrade path above.

@diasbruno
[fix] keep references of modals.
c638555 
keeping a reference of all modals in order to manage
when to add/remove classes and aria from elements
correctly.
@diasbruno diasbruno mentioned this pull request Mar 22, 2017
Closed
5 tasks
@diasbruno diasbruno requested a review from claydiffrient March 22, 2017 17:06
@diasbruno diasbruno added the bug label Mar 22, 2017
@diasbruno diasbruno added this to the 1.7 milestone Mar 22, 2017
AndersDJohnson
AndersDJohnson approved these changes Mar 22, 2017
View reviewed changes
Copy link
Contributor

@AndersDJohnson AndersDJohnson left a comment

Choose a reason for hiding this comment

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

@diasbruno Yes, we need this!

@boonier
Copy link

boonier commented Mar 23, 2017

@diasbruno lgtm thanks

@boonier
Copy link

boonier commented Apr 4, 2017

@diasbruno is this close to being merged in yet? 🙏 :)

@diasbruno diasbruno mentioned this pull request Apr 4, 2017
Closed
@AndersDJohnson
Copy link
Contributor

@diasbruno Please merge! 👍

@claydiffrient claydiffrient merged commit e579a0d into reactjs:v1 Apr 10, 2017
@AndersDJohnson
Copy link
Contributor

Let's get a new version released with this too!

@AndersDJohnson AndersDJohnson mentioned this pull request Apr 11, 2017
Closed
5 tasks
@boonier
Copy link

boonier commented Apr 12, 2017

👍 cheers

@claydiffrient claydiffrient mentioned this pull request Apr 13, 2017
Closed
@boonier
Copy link

boonier commented Apr 24, 2017

@claydiffrient is this in the 1.7.7 release?

@claydiffrient
Copy link
Contributor

Yes, it should be in and working in that release @boonier.

@boonier
Copy link

boonier commented Apr 24, 2017

Great thanks @claydiffrient

@diasbruno diasbruno deleted the feature/modal-reference-count branch June 14, 2017 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@claydiffrient claydiffrient claydiffrient approved these changes

@AndersDJohnson AndersDJohnson AndersDJohnson approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
1.7
Development

Successfully merging this pull request may close these issues.
4 participants
@diasbruno @boonier @AndersDJohnson @claydiffrient