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

Limit height of user page section on user profiles #2834

Merged
merged 8 commits into from Mar 29, 2018

Conversation

@nekodex
Copy link
Collaborator

@nekodex nekodex commented Mar 28, 2018

...'cuz some people like to play silly buggers

fixes #2815

@@ -80,6 +85,12 @@
width: 100%;
}

&__content-overflow-wrapper {
max-height: 400px;

This comment has been minimized.

@shortthu

shortthu Mar 28, 2018
Contributor

Why use 400px since the max height of most old userpage is 192px?

This comment has been minimized.

@Xinnoh

Xinnoh Mar 28, 2018

From what I see, the userpage doesn't have the option to expand and is locked at 400.
400 is approx the size of an expanded userpage on old web

@@ -35,6 +35,11 @@
padding: 0;
}

&--me-section {
padding-right: 0px;

This comment has been minimized.

@nanaya

nanaya Mar 28, 2018
Collaborator

hmmm

This comment has been minimized.

@nanaya

nanaya Mar 28, 2018
Collaborator

I think most scrollbars like this do follow the padding (as in not sticking to the edge).

nekodex added 5 commits Mar 29, 2018
…ight
@@ -97,5 +119,9 @@
right: 0;
top: 0;
cursor: move;

.@{top}--userpage & {
margin-right: 30px;

This comment has been minimized.

@nanaya

nanaya Mar 29, 2018
Collaborator

Apparently the margin is different between mobile and desktop. Also both have their own variables.

@nanaya
nanaya approved these changes Mar 29, 2018
@nanaya nanaya merged commit 859b348 into ppy:master Mar 29, 2018
2 checks passed
2 checks passed
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nekodex nekodex deleted the nekodex:limit-userpage-height branch Mar 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

5 participants