gapMode prop/API is not actually used
#65
Comments
|
Hey, thank you for the bug report. v3.8.0 has been released with a fix |
|
Holy cow, that was so fast!! Thanks a million @theKashey, you rock :) |
|
Gah actually, sorry to be annoying, but I'm not sure this is working. I updated the above CodeSandbox link (https://codesandbox.io/s/romantic-jennings-tv3l31?file=/demo.js:397-444) to use 3.8.0 and gapMode is still not working as expected, and instead there's a React error in the DOM which appears to imply that
|
|
😭 because problem is now on ReactRemoveScroll side. I need to update relationships between focus-on(supports) -> remove-scroll(🤡) -> remove-scroll-bar(supports). Works on local due some unpublished updates. I need to develop more reality-related test suites |
## Summary The `scrollLock` property on `EuiFocusTrap` "freezes" the width of the of the body so that the removal/addition of the scrollbar when a full screen focus trap opens (e.g. a modal, flyout, or other fullscreen mode) doesn't cause the page width to jump/rerender. It turns however, that this behavior (which sets a `margin-right` on the `body` for the width of the scrollbar) does not work as expected in Kibana due to its existing `width: 100%; min-width: 100%` CSS on its `body`. As such, we need to temporarily work around this by setting `padding-right` instead until theKashey/react-focus-on#65 is fixed. d59faae can be reverted if not desired in this PR. ### Checklist - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
|
@theKashey Hey! Any progress on the peer dependency to react-remove-scroll, and getting that up to speed? |
|
🫠 time flies. Sorry, too much other stuff fighting for priorities. |
|
There could be some glitches in the scrollbar if one uses |
Despite being documented in the main README, setting
gapMode="padding"as a prop does not appear to correctly change the CSS applied to the pagebodyto setpadding-rightinstead ofmargin-right(which the underlying react-remove-scroll-bar library implies it should).gapModeusage appears to be nonexistent, and its original implementation no longer exists in the codebase.Demo of broken behavior: https://codesandbox.io/s/romantic-jennings-tv3l31?file=/demo.js:397-444
Notice that even with
gapMode="padding"passed, when the focus lock is open, the CSS rendered is still using margin-right:A bit more context:
Our team could really use support for this, as unfortunately one of our major consumers has
width: 100%; min-width: 100%CSS set on their body, which causesmargin-rightto not work as expected. We don't really have standing to tell them to remove this CSS, so the only other alternative to usepadding-rightinstead (which works correctly with 100% width).As a workaround we're currently grabbing the
--removed-body-scroll-bar-sizeCSS var and manually settingpadding-right, but we'd love to be able to remove this workaround.The text was updated successfully, but these errors were encountered: