overflow: hidden is not removed from the component when it's opened at certain zoom levels
covertbert opened this issue ยท 29 comments
In my application in Google Chrome if I zoom out to 75% and set isOpened
to true overflow: hidden
is not removed from the component.
Edit: To add to this, I've narrowed it down to an adjacent button having padding in it and the only way to fix it seems to be to remove this padding entirely
+1 We have the exact same behavior in our application when zooming down to 75%! It makes it impossible for the user to use nested dropdowns for example as the content is cut by the overflow:hidden on container.
v5 was a complete rewrite, I believe it should not have this issue (not published as latest
though, but you can try it as @nkbt/react-collapse
)
Thanks, I'll give it a try. :)
When are you btw considering releasing v5 at latest?
I'm using this version for a while in several projects without any issues, but don't really have time (read "want") to fill up all the documentation/migration guide (though it is almost drop-in replacement) for it, which is the main blocker for release.
Ok I see. It is maybe a bit difficult to get help from outside for that task (?) Let know here if there is any way we can help to make it happen. We cannot on our end use a non-officially-released version in production you imagine :)
As new react-collapse@5.0.0
is released https://github.com/nkbt/react-collapse/releases/tag/v5.0.0 this issue has been most likely already solved.
Please feel free to re-open if the issue persists
Using react collapse 5.0.0 I can confirm that the issue persists.
overflow: hidden is set while the animation is going on, and set back to auto at the end. It calculates what will be the final height but because of it counting based on half-pixels because of devicePixelRatio it seems that it never finishes. Sรฅ overflow:hidden is then never removed.
I can reproduce everywhere react-collapse is used by using devicePixelRatio = 1.25.
On my very screen it is with a 110% zoom. For some of our testers it is whit 75% zoom, so it depend on your resolution. (When you zoom +/-, the devicePixelRatio changes, and it happens always with devicePixelRatio = 1.25)
To know which devicePixelRatio you have on your window, you can write window.devicePixelRatio in the console.
@nkbt Is it something you could try to reproduce?
Reopened. Thanks for the report ๐
You are welcome. So far i use that (dirty) workaround. It kills a bit the animation but ensures the user can access the functionality. I could right now reproduce with deviceRatio 2.20, so I am unsure what exactly affects it.
handleReactCollapsePixelRatioIssue = (): void => {
try {
// gets all the react-collapse elements within filter
const filterNode = ReactDOM.findDOMNode(this.filterOuterWrapperRef.current);
const collapseContainers = (filterNode as HTMLElement).getElementsByClassName('ReactCollapse--collapse');
// resets height and overflow
for (let i = 0; i < collapseContainers.length; i++) {
(collapseContainers[i] as HTMLElement).style.height = 'auto';
(collapseContainers[i] as HTMLElement).style.overflow = 'initial';
}
} catch (e) {
log('Filter: react-collapse fix for zoom level failed', e);
}
};
I pass handleReactCollapsePixelRatioIssue to the onRest method provided by react-collapse. I also call it with a listener on deviceRatio change, which is initialised on mount. See below:
initPixelRatioMonitoring = () => {
this.handleReactCollapsePixelRatioIssue();
matchMedia('(resolution: 2.20dppx)').addListener(this.handleReactCollapsePixelRatioIssue);
};
Hey! In one case we have a similar issue ๐ .
We are rendering a page with an opened collapse component and the content of the collapse is hidden.
It's a floating issue and reproduces only in one case but without any zooming or scaling the page.
react-collapse
version is 5.0.1
Browser: chrome 85.0.4183.83.
Tried pretty hard and cannot reproduce in my set of examples. Would really appreciate if you can create a codepen with an example that breaks.
This is still a issue for us.
It feels like some sort of rounding error based on pixels since if we add 2-3px of padding when it happens the issue is resolved.
Otherwise a timeout prop would solve the issue :D But I guess you don't want to re-introduce that.
Yeah that must be some rounding issue. I cannot reproduce it myself, so cannot fix the issue for good.
- What are the parameters of the env you can reproduce it? System, browser, screen, resolution, zoom level, window size, full screen/windowed, videocard, drivers, etc. also specific components (some jsbin)
combination of these might render that odd pixel.
For example, we could solve it by some rounding like 1-2px would be considered "done", but only after debounce "step" (so we can tell it is stable).
But that might be an issue by itself, because the case when we collapse element without hiding from 1-2px to auto is quite legit. Which could be solved by letting you choose "settle tolerance" option or smth
tldr: it's complicated :(
Just got reports from my colleagues about this issue. Their vision is degrading and they zoom in a lot of websites, ours including.
After a little digging I noticed that computed height of the element is always larger than the height we set. And zoom in/out skews the difference even more. So it may happen
in the mdn doc on clientHeight I found this:
Note: This property will round the value to an integer. If you need a fractional value, use element.getBoundingClientRect().
@okhomenko should we just do Math.floor()
and give it 1px tolerance for both checks?
As in
isFullyOpened = isOpened && Math.abs(contentHeight - containerHeght) <= 1
Bounding rect is kind of slow to use, I would rather sacrifice that precision for speed...
Reproducible jsfiddle: https://jsfiddle.net/d5knv0ym/45/
Mac, Monterey, Chrome 96.0.4664.55 (Official Build) (x86_64)
@okhomenko should we just do
Math.floor()
and give it 1px tolerance for both checks?As in
isFullyOpened = isOpened && Math.abs(contentHeight - containerHeght) <= 1
Bounding rect is kind of slow to use, I would rather sacrifice that precision for speed...
I like the idea with 1px tolerance
@okhomenko which browser are you using because I cannot reproduce the issue in Opera with 110% zoom anyway =)
Mac, Monterey, Chrome 96.0.4664.55 (Official Build) (x86_64)
Gotcha. Missed it in the prev message, sorry. I checked in chrome and it indeed has the issue.
I assume each browser has it's own math for computing client heights/width, based on DPI, zoom level etc.
Thanks for looking into this and lmk if I can help
If you can try to reproduce the issue with updated https://nkbt.github.io/react-collapse/bundle.js
that would be great!
When I use bundle.js instead of react-collapse.min.js I get this error: "Uncaught ReferenceError: ReactCollapse is not defined"
.
Ah ok, forgot that the demo site has example baked in and does not export ReactCollapse.
My testing did not show any downsides to that solution so I will publish patch version and lets see how it goes then
That's awesome, thanks so much @nkbt. I can test it right away on my dev environment once package is published
Published react-collapse@5.1.1
@nkbt tested on my environment and everything works amazing.
Thanks a lot. My colleagues much appreciate your hustle ๐
@okhomenko amazing! thank you very much for the detailed report, that made a world of difference