-
Notifications
You must be signed in to change notification settings - Fork 810
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
[changed] use object className and overlayClassName prop to override … #330
Conversation
hey @cassln, this PR looks good, but why would you want to replace the class names? sass or less style (replace .{my-class}.react-modal-base { ... }
.{my-class}.react-modal {
.react-modal-overlay { ... }
.react-modal-content { ... }
} |
Hey @diasbruno. I want to replace the class names because of I'm sticking BEM notation. And cascades like Also, this feature makes possible getting clean markup for react-modal in result. :) |
What about merge the feature? :) I really need this in working project. |
@cassln don't wait, use it from your fork. :) |
Be patient. I do think this PR is good. there is some work going on to get a version in between |
lib/components/ModalPortal.js
Outdated
base: PropTypes.string.isRequired, | ||
afterOpen: PropTypes.string.isRequired, | ||
beforeClose: PropTypes.string.isRequired | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that only the base
of the classes change. Maybe we can short this changes by keeping the suffixes unchanged like {ReactModal}_before-open
and {ReactModal}_after-open
. Any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The suffixes are not flexible. There are BEM alternative naming schemes. The suffixes make impossible of using them. Also, we can pass props like this (without BEM example):
{
base: 'modal',
afterOpen: 'open',
beforeClose: 'close'
}
Do you have ability to release version between 1.6.5 and 2.0. This It would be very helpful. 👍 |
@cassln 1.7.0, 1.7.1, and 1.7.2 have been released. |
…the default content and overlay classes; use bodyOpenClassName to override body class name when modal opened
The branch has been rebased. |
@cassln @diasbruno Is there a chance to this get merged? I really need the |
cc @claydiffrient @cassln |
@cassln @diasbruno Hi everyone! Any update about this pl? In my project we are using this lib and we really need can change the body classname |
Why this PR is not merged yet ? |
This is a backport of @cassln's feature to version 1.8.x. Original PR is reactjs#330. reactjs#330
This is a backport of @cassln's feature to version 1.8.x. Original PR is reactjs#330. reactjs#330
This is a backport of @cassln's feature to version 1.8.x. Original PR is reactjs#330. reactjs#330
This is a backport of @cassln's feature to version 1.8.x. Original PR is reactjs#330. reactjs#330
This is a backport of @cassln's feature to version 1.8.x. Original PR is #330. reactjs/react-modal#330
This is a backport of @cassln's feature to version 1.8.x. Original PR is #330. reactjs/react-modal#330
This is a backport of @cassln's feature to version 1.8.x. Original PR is #330. reactjs/react-modal#330
Changes proposed:
Upgrade Path (for changed or removed APIs):
For overriding default content and overlay classes you can pass object
with three required properties:
base
,afterOpen
,beforeClose
.Old behavior is saved, when passed a string.
Acceptance Checklist:
CONTRIBUTING.md
.