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.

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

[added] Allow overlay override #319

Closed
wants to merge 18 commits into from
Closed

[added] Allow overlay override #319

wants to merge 18 commits into from

Conversation

cema-sp
Copy link

@cema-sp cema-sp commented Jan 30, 2017 •
edited
Loading

Continues and Closes #281

@diasbruno @claydiffrient Sorry that it took so long for me to prepare this feature. Leaving previous branch if someone uses it.

To override overlay one should provide overlay prop to Modal containing react element. That component will be cloned and filled with overlay props and children (using React.cloneElement).

Another way may be to provide overlayComponent and overlayProps and then create react element from them (using React.createElement). Which way us better and more idiomatic?

Changes proposed:

  • Extract ModalOverlay to component
  • Allow user to provide custom overlay with Modal.props.overlay prop

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.

claydiffrient and others added 13 commits December 31, 2016 14:49
@claydiffrient
Add Linting (#293)
596f423 
* Add linting

* Make specs pass linter

* Make lib/helpers pass linter

* Make lib/components pass linter

This also does some signfiicant refactoring of the code
to appease the linter's pro-ES2015+ stance.

closes #289
closes #286

* Make travis run the lint task as well as specs

closes #284
@claydiffrient
Add code climate and code coverage
a09ff8d
@claydiffrient
Add language for CC duplication engine
721cc1e
@claydiffrient
Setup coveralls and fix some code climate stuff
3d6142c
@claydiffrient
Add badges to readme
e10181d
@hiroppy @claydiffrient
[fixed] Fix this scope (#296)
d50918c 
* [fixed] Make use of es6 modules

* [fixed] Fix `this` scope
@claydiffrient
Add sauce labs support
b261459
@hiroppy @claydiffrient
Get rid of CommonJS module (#299)
a776118
@hiroppy @claydiffrient
Modify the sample code to es2015 syntax in README.md (#295)
8108173
@jamesongamble @claydiffrient
Updates License (#303)
ad9e115
@claydiffrient
Add cross browser testing via SauceLabs (#298)
a1287bc 
* Add sauce labs testing info to karma

This also removes Node versions 4, 5, 6 from Travis.  The node
version only matters for development not for use.  I don't
think it's a problem officially supporting only the latest node
for development.

* Make specs work under IE 11
@claydiffrient
Add sauce labs reporter for better SL statuses
0dc486a
@claydiffrient
[fixed] Infinite loop in tab scoping
75a1aa3
@diasbruno
Copy link
Collaborator

@cema-sp this looks awesome. i'll bring some feedback later.

oisu and others added 2 commits February 1, 2017 13:37
@diasbruno diasbruno mentioned this pull request Feb 16, 2017
Closed
5 tasks
diasbruno
diasbruno reviewed Feb 16, 2017
View reviewed changes
Copy link
Collaborator

@diasbruno diasbruno left a comment

Choose a reason for hiding this comment

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

Awesome PR. It just need to squash all commits. I'll try to test more this feature.

mgh and others added 3 commits February 18, 2017 15:56
@mgh @diasbruno
[fixed] respect closeTimeoutMS during unmount
ba2fe90 
Fixes issue #17: "When the modal is unmounted, it will abruptly close,
not waiting for any animations to finish."

The bug was caused by the Modal component unmounting the portal
immediately in `componentWillUnmount` regardless of whether the portal
is currently closing or would animate to close.

Now when a Modal has a non-zero `closeTimeoutMS`, the Modal inspects the
portal state before closing.  If the portal is open or closing, but not
closed, it waits to unmount the portal.  If the portal is open and not
already closing, the Modal calls closeWithTimeout() to trigger the
close.

Adds test to ensure portal DOM persists after Modal is unmounted when
the Modal has a `closeTimeoutMS` set.  Updates existing tests to
properly unmount test Modal instances.
@diasbruno
[chore] use afterEach to cleanup modals automatically.
a8b7aa0
@cema-sp
Add simple ModalOverlay component
d7193a8 
* Allow overlay override
* Add overlay overriding info
@coveralls
Copy link

Coverage Status

Coverage increased (+2.4%) to 83.072% when pulling d7193a8 on cema-sp:modal-overlay into a8b7aa0 on reactjs:master.

@cema-sp
Copy link
Author

cema-sp commented Feb 27, 2017

@diasbruno Rebased & squashed

@diasbruno diasbruno added this to the 2.0.0 milestone Mar 2, 2017
@diasbruno diasbruno removed this from the 2.0.0 milestone Jun 15, 2017
lindskogen
lindskogen reviewed Jul 7, 2017
View reviewed changes
examples/basic/app.js
},

openModal: function() {
this.setState({modalIsOpen: true});
this.setState({ ...this.state, modalIsOpen: true });
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there some special reason for this? setState actually does shallow merge automatically.

@Monte9
Copy link

Monte9 commented Sep 25, 2017

@diasbruno would this PR get merged at any point soon? Would love to have the scroll functionality on the modal. thanks.

@rifkegribenes
Copy link

@Monte9 FYI if all you need is a scrollable modal, you can just add

.ReactModal__Overlay {
  overflow-y: scroll !important;
 }

to your css

@diasbruno
Copy link
Collaborator

@rifkegribenes that's correct. the idea for this PR is to make the overlay customizable.

@cema-sp I think it's a good time to 'revive' this branch. Any help you need to get this branch updated, please let me know.

@hszeto
Copy link

hszeto commented Feb 23, 2018

@rifkegribenes thanks for the quick css fix.

I found the following works better for my case. It retains the modal positon. (I have the modal open a little closer to the top instead of center.)

.ReactModal__Overlay--after-open {
  overflow-y: scroll !important;
}

@cema-sp
Copy link
Author

cema-sp commented Mar 18, 2020

Sorry, won't be able to get back to it any time soon.

@cema-sp cema-sp closed this Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diasbruno diasbruno diasbruno left review comments

@lindskogen lindskogen lindskogen left review comments

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.