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

Console error "cannot calculate scrollbar size because it is removed" #35

Closed
raymond-shanley-dynatrace opened this issue Aug 29, 2022 · 11 comments
Closed

Console error "cannot calculate scrollbar size because it is removed" #35

raymond-shanley-dynatrace opened this issue Aug 29, 2022 · 11 comments
Assignees
@theKashey

Comments

@raymond-shanley-dynatrace
Copy link

When an app XYZ is using a library that in turn is using react-remove-scroll-bar and at the same time, the XYZ app has the scrollbar removed for a different reason that can't be avoided.

So the app has something like the following:

<body style="overflow: hidden;">

Then a console.error is logged in the console.

Can we avoid logging this error?
@explorador
Copy link

This is the commit that added this message. @theKashey what's the purpose of this? Thanks for your help!

@theKashey
Copy link
Owner

The error message is displayed only during development and should cause any trouble for a real customer, and I can replace the error with a warning if that would work for you.

The change itself was a fix for theKashey/react-remove-scroll#71 (issues are often raised for library consumers) and logging was added to reduce "wtf per minute" - the most important metric I have.

@explorador
Copy link

Makes sense. I think a warning is more fitting too. Thanks for your help!

@theKashey theKashey self-assigned this Sep 15, 2022
@hixus
Copy link

hixus commented Sep 22, 2022 •
edited
Loading

Not completely aware how react-remove-scroll-bar does it's magic but should it somehow track with css class or something that if it has already hidden the scrollbar? Then it would not matter if react-remove-scroll-bar would be initialised multiple times.

@theKashey
Copy link
Owner

It tracks "own" classes but is not able to handle third-party ones. Can only report an issue

@saradevh
Copy link

saradevh commented Oct 3, 2022

@theKashey Hi! Do you still plan to update the error with a warning? Thanks

@theKashey
Copy link
Owner

Absolutely.

@hixus
Copy link

hixus commented Oct 5, 2022 •
edited
Loading

It tracks "own" classes but is not able to handle third-party ones. Can only report an issue

No I mean if react-remove-scroll-bar component is mounted twice each of them executes getGapWidth => getOffset which has the error mentioned in this ticket. And the latter will always fail as react-remove-scroll-bar itself sets body as overflow hidden.

Therefore I was thinking it could set react-remove-scroll-bar-initialized css class, etc. to body and if it's set the later instances would know not to do anything. Context provider could be another solution.

@kof
Copy link

kof commented Oct 5, 2022 •
edited
Loading

It would make sense to allow disabling that warning, because I have overflow: hidden on body and having that warning on every interaction with radix-ui components. It's a transitive dependency for me, so I would need a way to disable it from "outside".

radix-ui/primitives#1593 (comment)

@theKashey
Copy link
Owner

No I mean if react-remove-scroll-bar component is mounted twice each of them executes getGapWidth => getOffset which has the error mentioned in this ticket.

😕 this should not happen, and that's definitely a bug.
Let's address this in a simple way - completely removing noise now and thinking about better ways to expose potential problems with calculations later.

theKashey added a commit that referenced this issue Oct 10, 2022
@theKashey
fix: remove measure time error reporting, fixes #35
c8937c9
@theKashey theKashey mentioned this issue Oct 10, 2022
Closed
theKashey added a commit that referenced this issue Oct 12, 2022
@theKashey
Merge pull request #37 from theKashey/fix-console-notification
86d55a7 
fix: remove measure time error reporting, fixes #35
@theKashey
Copy link
Owner

theKashey commented Oct 12, 2022 •
edited
Loading

Particularly this issue has been resolved.
You need to update to version 2.3.4, I am also going to publish v2.4.0 in a few hours to address other upstanding issues.

Version update propagation might take a while and if you need results here and now - you will need to find react-remove-scroll-bar in your lock file and update(remove) the corresponding record to let your package manager install a fresh version.

Yarn users can do npx yarn-unlock-file all --only "react-remove-scroll-bar" followed by yarn to perform a simular operation without manual intervention.

@andy-hook andy-hook mentioned this issue Oct 19, 2022
Merged
istarkov added a commit to webstudio-is/webstudio that referenced this issue Dec 2, 2022
@istarkov
tired of this theKashey/react-remove-scroll-bar#35
1ac4c80
@yuhang1995 yuhang1995 mentioned this issue Jan 4, 2023
Closed
3 tasks
@takanome-dev takanome-dev mentioned this issue Mar 13, 2023
Closed
2 tasks
@Heenawter Heenawter mentioned this issue Mar 20, 2023
Merged
5 tasks
@theKashey theKashey mentioned this issue Apr 30, 2023
Closed
@abbazabacto abbazabacto mentioned this issue Sep 19, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@theKashey theKashey

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Improve scroll behavior theKashey/react-remove-scroll-bar
6 participants
@kof @theKashey @hixus @explorador @saradevh @raymond-shanley-dynatrace