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

Refinement #30

Merged
merged 1 commit into from
May 14, 2022
Merged

Refinement #30

merged 1 commit into from
May 14, 2022

Conversation

michael-o
Copy link
Member

@kwin Here is another try. This fits now into the sidebar, but will also narrow it for the topbar. I think those 7 percert are acceptable for the topbar.

@michael-o michael-o requested a review from hboutemy May 14, 2022 15:02
@kwin
Copy link
Member

kwin commented May 14, 2022

I don't think you get centered alignment with just width in percentage. This is due to the fact that the margins of the sidebar are fix, while the column overall is responsive, i.e. for very wide browser windows even with this PR the search input is misaligned and the left margin is much wider than the fix right one.
Screenshot 2022-05-14 at 17 09 34

@michael-o
Copy link
Member Author

I don't think you get centered alignment with just width in percentage. This is due to the fact that the margins of the sidebar are fix, while the column overall is responsive, i.e. for very wide browser windows even with this PR the search input is misaligned and the left margin is much wider than the fix right one. Screenshot 2022-05-14 at 17 09 34

I agree. Therefore, I likely need to revert my previous commit and leave the ticket open. My CSS knowledge is to weak in this regard.

@kwin
Copy link
Member

kwin commented May 14, 2022

I just tried width: calc(100% - 28px); and this worked fine. Look also at https://stackoverflow.com/questions/17631611/100-width-minus-margin-and-padding and https://caniuse.com/calc.

@michael-o
Copy link
Member Author

I just tried width: calc(100% - 28px); and this worked fine. Look also at https://stackoverflow.com/questions/17631611/100-width-minus-margin-and-padding and https://caniuse.com/calc.

Where do those 28px come from?

@kwin
Copy link
Member

kwin commented May 14, 2022

Where do those 28px come from?

Right and left margin of 14px in the sidebar. One could even restrict that width to only inside the sidebar.

@michael-o
Copy link
Member Author

Where do those 28px come from?

Right and left margin of 14px in the sidebar. One could even restrict that width to only inside the sidebar.

Double check when this search bar is in the top bar. Physics change there.

@kwin
Copy link
Member

kwin commented May 14, 2022

You can easily restrict width: calc(100% - 28px); to only sidebar with a selector like .sidebar-nav input.search-query.

@michael-o
Copy link
Member Author

Yes, this is what I thought too.

@kwin
Copy link
Member

kwin commented May 14, 2022

Maybe 28px is not correct.
The important metrics are:

margin-right and left: 9px and border-radius: 15px

@kwin
Copy link
Member

kwin commented May 14, 2022

Looks like 30px = 2 x border-radius is the correct value to subtract here.

@michael-o michael-o marked this pull request as ready for review May 14, 2022 17:09
Copy link
Member

@kwin kwin left a comment

Choose a reason for hiding this comment

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

LGTM now. Thanks.

Second try, make it dynamic in the sidebar only.

This closes #30
@asfgit asfgit merged commit 6ab7917 into master May 14, 2022
@michael-o michael-o deleted the MSKINS-173-2 branch May 14, 2022 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants