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

[dashboard] Remove margin-bottom from PageWithSubMenu #4417

Merged
merged 1 commit into from
Jun 16, 2021

Conversation

corneliusludmann
Copy link
Contributor

@corneliusludmann corneliusludmann commented Jun 7, 2021

Avoids unnecessary scrollbars. Fixes #3859.

Before this change, pages in the settings section do have a margin at the bottom. That leads to unnecessary scrollbars:

image

Without the margin, the scrollbars disappear:

image

Is there any good reason for the margin?

(Examples are with a canvas size of 1440x1049px.)

@JanKoehnlein
Copy link
Contributor

JanKoehnlein commented Jun 8, 2021

/werft run

👍 started the job as gitpod-build-clu-horizontal-scrollbar-3859.1

@JanKoehnlein JanKoehnlein requested review from JanKoehnlein and removed request for jankeromnes June 8, 2021 08:36
@jankeromnes
Copy link
Contributor

jankeromnes commented Jun 11, 2021

Is there any good reason for the margin?

FYI I believe this was to prevent awkward scrolling behavior when there are drop-downs / context menus near the bottom of the page (where the page height / scroll position abruptly changes depending on collapsed state). I don't remember where that happened though.

@corneliusludmann
Copy link
Contributor Author

FYI I believe this was to prevent awkward scrolling behavior when there are drop-downs / context menus near the bottom of the page (where the page height / scroll position abruptly changes depending on collapsed state). I don't remember where that happened though.

OK, thanks for the comment. So, I think we should merge this change and when the original concern occurs again, we should find a proper solution (and in doubt document the decision next to the code). What do y'all think?

@JanKoehnlein
Copy link
Contributor

JanKoehnlein commented Jun 16, 2021

/werft run

👍 started the job as gitpod-build-clu-horizontal-scrollbar-3859.2

Copy link
Contributor

@JanKoehnlein JanKoehnlein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@corneliusludmann corneliusludmann merged commit f3b6cbe into main Jun 16, 2021
@corneliusludmann corneliusludmann deleted the clu/horizontal-scrollbar-3859 branch June 16, 2021 15:12
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.

Horizontal scrollbar on pages that shouldn't have one
4 participants