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

Modals no longer use .modal-open to affect the <body> scroll #33551

Merged
merged 1 commit into from
May 13, 2021
Merged

Modals no longer use .modal-open to affect the <body> scroll #33551

merged 1 commit into from
May 13, 2021

Conversation

GeoSot
Copy link
Member

@GeoSot GeoSot commented Apr 4, 2021 •
edited
Loading

.modal-open.modal css rules, overrides default .modal rules, concerning overflow property

image

preview

@GeoSot GeoSot added the css label Apr 4, 2021
@GeoSot GeoSot requested a review from a team as a code owner April 4, 2021 21:29
@GeoSot GeoSot requested review from ffoodd and mdo April 13, 2021 10:03
@GeoSot GeoSot force-pushed the gs-remove-modal-css-override branch from 28055b1 to 54cc71e Compare April 20, 2021 20:20
@GeoSot GeoSot mentioned this pull request Apr 20, 2021
Closed
@GeoSot GeoSot force-pushed the gs-remove-modal-css-override branch from 54cc71e to 7381417 Compare May 7, 2021 13:34
@mdo mdo changed the title Remove modal css rules override about overflow Modals no longer use .modal-open to affect the <body> scroll May 10, 2021
@GeoSot GeoSot removed the v4 label May 10, 2021
mdo
mdo approved these changes May 10, 2021
View reviewed changes
Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

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

Looks good to me—@ffoodd wouldn't mind your eyes.

@GeoSot GeoSot force-pushed the gs-remove-modal-css-override branch from 7381417 to 1a1175d Compare May 12, 2021 10:18
@mdo mdo merged commit 130a5ba into main May 13, 2021
@mdo mdo deleted the gs-remove-modal-css-override branch May 13, 2021 15:55
@raduchiriac
Copy link
Contributor

Shouldn't you remove this as well https://github.com/twbs/bootstrap/blob/main/js/src/modal.js#L127 as it is useless I guess?

@GeoSot
Copy link
Member Author

GeoSot commented May 17, 2021

Was meant to be done on #33705 but it is considered as a breaking change

@angieEncoded
Copy link

Can docs be clarified that this class doesn't exist anymore? I was trying to use it on its own for something which resulted in some confusion until digging up this thread.

https://getbootstrap.com/docs/5.0/components/modal/#usage

@kyletsang kyletsang mentioned this pull request Jun 3, 2021
Closed
This was referenced Jun 12, 2021
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdo mdo mdo approved these changes

@ffoodd ffoodd Awaiting requested review from ffoodd

Assignees
No one assigned
Labels
css v5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@GeoSot @raduchiriac @angieEncoded @mdo