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.

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

Fix: Allow sidebars to scroll independently of main window #552

Conversation

adamphillips
Copy link

Had a go at resolving #500

There are a bunch of different ways of doing this kind of thing so I thought I'd suggest one and get some feedback.

This seemed like the least intrusive way of making the change but I'm happy to try again in a different way if someone can point me in the direction you'd prefer.

When the length of the sidebar exceeded that of the viewport, the body
would scroll. This lead to unusual scroll behaviour in the console as
both the console container and the main body would scroll at the same
time.

Resolve by fixing the height of the sidebars to 100% and allowing them
to scroll if necessary. This means it is never necessary for the body to
scroll and so the two scrolling areas now feel independent of each
other.

Closes Snapmaker#500
@parachvte
Copy link
Contributor

parachvte commented Nov 13, 2020

Nice try on the sidebar 😃
We have primary sidebar and secondary sidebar both may causing the overscroll.

@adamphillips
Copy link
Author

adamphillips commented Nov 13, 2020

Ah ok. I had seen the primary sidebar but didn't realise it could go that large.
Shall I apply a similar fix to the primary sidebar as well then?

@jane-rose
Copy link
Contributor

Cool, nice try! I will send a pr to fix the bug.

@parachvte
Copy link
Contributor

Ah ok. I had seen the primary sidebar but didn't realise it could go that large.

Widgets can be dragged between 2 bars.

Seems like @jane-rose has found another issue here (relates to mouse event handling).
You can discuss the issue with him, or check the option to allow maintainer to modify so we can make changes on your code.

@parachvte parachvte requested a review from jane-rose November 13, 2020 10:55
@adamphillips
Copy link
Author

Right ok.

@jane-rose I am happy to carry on working on it if you want to point me in the direction you want to go. Otherwise the option to allow edits is also checked so feel free to make the changes yourself if you prefer.

Thanks!

@jane-rose
Copy link
Contributor

jane-rose commented Nov 13, 2020

#557. Thanks, u have to preventDefault to make the event not bubble to the document

@adamphillips
Copy link
Author

Great, shall I close this PR then in favour of the newer one?

@jane-rose
Copy link
Contributor

Definitely u can

@jane-rose jane-rose closed this Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants