-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fixed/Build from Source’ tab partially hidden in mobile view on Install page #2106 #2108
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
base: gh-pages
Are you sure you want to change the base?
fixed/Build from Source’ tab partially hidden in mobile view on Install page #2106 #2108
Conversation
dscho
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!
@To1ne what do you say?
|
when you will merge it sir , |
To1ne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kushkumarkashyap7280 I've got some comments. Let me know what you think.
| font-size: 0.75rem; | ||
| line-height: 1.2em; | ||
| display: flex; | ||
| flex: 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kushkumarkashyap7280 This makes the tab buttons grow on semi-small screens:
vs the old
Is that intentional?
On large'ish screen we always show tab buttons that fit the contentd;
Why have them grow on mobile? As far as I can tell, it has no effect on the issue you're trying to fix.
| a[role="tab"] { | ||
| padding: .5rem; | ||
| padding: .5rem .25rem; | ||
| font-size: 0.75rem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's arguable, but I think it's too small. I rather have you drop this line out.
| @media (max-width: $mobile-m) { | ||
| a[role="tab"] { | ||
| padding: .5rem; | ||
| padding: .5rem .25rem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change is all we need.
Until you want to take it one step further and support really narrow screens. But in that case I suggest we work on putting flex-direction: column on .install-tabs, so we have vertical tabs. But there's more work to be done to have it look nicely, because for example round corners aren't okay:
(and it would also be better to not have "Install" and "Latest version" side-by-side.
…ll page #2106
screenshots
previously
fixed
Changes
font size and padding reduced so clearly visible on phone view
edit install.css file for mobile view