-
Notifications
You must be signed in to change notification settings - Fork 45
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
Modal | JS | Add page scrolling option #1304
Changes from 2 commits
2584e00
90071df
5a731b0
29b622f
353af95
5904236
5b6ab74
702200a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,7 +58,10 @@ properties: | |
uuid: | ||
type: string | ||
description: Unique ID for modal, randomly generated if not provided. | ||
|
||
prevent_body_scroll: | ||
type: boolean | ||
description: Prevent background page content from scrolling when modal is open | ||
hidden: true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sort of a nitpick, but can we make the default value (false) explicit the schema? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although, now that I think about it, do we ultimately want it to actually default to I suppose the way you have it is the path of least resistance though. Don't let me derail it. |
||
# @todo: persistent and hide close button props are not ready. | ||
# persistent: | ||
# type: boolean | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,7 @@ bolt-modal:not([ready]) { | |
position: fixed; | ||
top: 0; | ||
left: 0; | ||
width: 100vw; | ||
width: 100%; // Use % instead of vh or modal scrollbar is hidden behind bold scrollbar in IE11 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 for commenting on this |
||
height: 100vh; | ||
pointer-events: none; | ||
transition: opacity $bolt-modal-transition; | ||
|
@@ -419,13 +419,3 @@ bolt-modal:not([ready]) { | |
.c-bolt-modal__dialog-title { | ||
@include bolt-visuallyhidden; | ||
} | ||
|
||
// https://davidwalsh.name/detect-scrollbar-width | ||
// @todo: refactor into core JS/CSS | ||
.c-bolt-modal__scrollbar-measure { | ||
position: absolute; | ||
top: -9999px; | ||
width: 100px; | ||
height: 100px; | ||
overflow: scroll; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
/** | ||
* Determine whether or not the BODY has a visible scrollbar | ||
* @returns {Boolean} - Returns true if element has scrollbar | ||
*/ | ||
export const bodyHasScrollbar = () => { | ||
const bodyRect = document.body.getBoundingClientRect(); | ||
return bodyRect.left + bodyRect.right < window.innerWidth; | ||
}; | ||
|
||
// https://davidwalsh.name/detect-scrollbar-width | ||
/** | ||
* Get scrollbar width by temporarily adding element with scrollbar to page then removing it | ||
* @returns {number} - Width of the scrollbar in pixels, without unit, e.g 15 | ||
*/ | ||
export const getScrollbarWidth = () => { | ||
const scrollDiv = document.createElement('div'); | ||
scrollDiv.className = 'c-bolt-modal__scrollbar-measure'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sure to remove this ;) |
||
scrollDiv.style.cssText = | ||
'position: absolute; top: -9999px; width: 100px; height: 100px; overflow: scroll;'; | ||
document.body.appendChild(scrollDiv); | ||
const scrollbarWidth = | ||
scrollDiv.getBoundingClientRect().width - scrollDiv.clientWidth; | ||
document.body.removeChild(scrollDiv); | ||
|
||
return scrollbarWidth; | ||
}; | ||
|
||
// Technique inspired by Bootstrap Modal: https://github.com/twbs/bootstrap/blob/master/js/src/modal/modal.js | ||
/** | ||
* Set padding on a given element equal to the browser's scrollbar width, used to prevent content shifting when scrollbars are turned off, e.g. when a modal opens | ||
* @param {HTMLElement} element - The target element | ||
* @param {(string\|number)} scrollbarWidth - The scrollbar width, with or without unit, e.g. 15 or 15px | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👏 we need to start doing this for all net-new JS coming in. Nice!! |
||
*/ | ||
export const setScrollbarPadding = (element, scrollbarWidth) => { | ||
if (!element) { | ||
return; | ||
} | ||
|
||
const originalPaddingRight = element.style.paddingRight; | ||
const calculatedPadding = window.getComputedStyle(element)['padding-right']; | ||
|
||
// Save original padding value for later | ||
element.originalPaddingRight = originalPaddingRight; | ||
sghoweri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
element.style.paddingRight = | ||
parseFloat(calculatedPadding) + scrollbarWidth + 'px'; | ||
}; | ||
|
||
/** | ||
* Reset padding on a given element, will remove current padding style and restore original padding style if necessary, used after modal closes | ||
* @param {HTMLElement} element - The target element | ||
*/ | ||
export const resetScrollbarPadding = element => { | ||
if (!element) { | ||
return; | ||
} | ||
|
||
if (typeof element.originalPaddingRight === 'undefined') { | ||
element.style.paddingRight = ''; | ||
} else { | ||
element.style.paddingRight = element.originalPaddingRight; // Restore original padding value | ||
} | ||
}; |
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.
@danielamorse would
no_body_scroll
make more sense here?