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

modal: don't add margin & padding when sticky is not full width #30621

Merged
merged 5 commits into from
Feb 23, 2021
Merged

modal: don't add margin & padding when sticky is not full width #30621

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a788134
modal: don't add margin & padding when sticky is not full width
muhamadamin1992 Apr 20, 2020
4c52f0c
Update modal.spec.js
XhmikosR Nov 20, 2020
f4fef6c
Check if element is shorter than window
rohit2sharma95 Dec 7, 2020
a0b7db4
Merge branch 'main' into modal-with-sticky
rohit2sharma95 Feb 22, 2021
af11a56
Merge branch 'main' into modal-with-sticky
XhmikosR Feb 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions js/src/modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,10 @@ class Modal extends BaseComponent {
_setElementAttributes(selector, styleProp, callback) {
SelectorEngine.find(selector)
.forEach(element => {
if (element !== document.body && window.innerWidth > element.clientWidth + this._scrollbarWidth) {
return
}

const actualValue = element.style[styleProp]
const calculatedValue = window.getComputedStyle(element)[styleProp]
Manipulator.setDataAttribute(element, styleProp, actualValue)
Expand Down
24 changes: 24 additions & 0 deletions js/tests/unit/modal.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,30 @@ describe('Modal', () => {
modal.toggle()
})

it('should not adjust the inline margin and padding of sticky and fixed elements when element do not have full width', done => {
fixtureEl.innerHTML = [
'<div class="sticky-top" style="margin-right: 0px; padding-right: 0px; width: calc(100vw - 50%)"></div>',
Copy link
Collaborator

Choose a reason for hiding this comment

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

While debugging I found that the test was failed because the clientWidth of the element was greater than window.innerWidth. So I changed it to width: calc(100vw - 50%). I am not sure about it, seems like a hack to me 😄

'<div class="modal"><div class="modal-dialog"></div></div>'
].join('')

const stickyTopEl = fixtureEl.querySelector('.sticky-top')
const originalMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10)
const originalPadding = Number.parseInt(window.getComputedStyle(stickyTopEl).paddingRight, 10)
const modalEl = fixtureEl.querySelector('.modal')
const modal = new Modal(modalEl)

modalEl.addEventListener('shown.bs.modal', () => {
const currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10)
const currentPadding = Number.parseInt(window.getComputedStyle(stickyTopEl).paddingRight, 10)

expect(currentMargin).toEqual(originalMargin, 'sticky element\'s margin should not be adjusted while opening')
expect(currentPadding).toEqual(originalPadding, 'sticky element\'s padding should not be adjusted while opening')
done()
})

modal.show()
})

it('should ignore values set via CSS when trying to restore body padding after closing', done => {
fixtureEl.innerHTML = '<div class="modal"><div class="modal-dialog"></div></div>'
const styleTest = document.createElement('style')
Expand Down