-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix the top bar going outside the window #33752
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -140,7 +140,6 @@ function SearchBar() { | |
| position: relative; | ||
| flex: 4; | ||
| flex-shrink: 1; | ||
| min-width: calc(${props => props.theme.space[7]}px * 2); | ||
| height: 100%; | ||
| border: 1px ${props => props.theme.colors.buttons.border.border} solid; | ||
| border-radius: ${props => props.theme.radii[2]}px; | ||
|
|
@@ -185,7 +184,9 @@ function SearchBar() { | |
| const Input = styled.input` | ||
| height: 38px; | ||
| width: 100%; | ||
| min-width: calc(${props => props.theme.space[9]}px * 2); | ||
| // min-width causes the filters and the actual input text to be broken into | ||
| // two lines when there is no space | ||
| min-width: calc(${props => props.theme.space[8]}px * 2); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume this had to be dropped from 9 to 8 to make everything fit? I remember setting it to a specific width (but I don't remember why because I didn't leave a comment 🙃), probably for both filters to fit on a single line even when the window is at its narrowest. But at the smallest window width, it's probably better to make the top bar usable rather than to keep both filters in the same line.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, when the width was 9, the content of the input was a little too wide.
👍 |
||
| background: transparent; | ||
| color: inherit; | ||
| box-sizing: border-box; | ||
|
|
||
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 seems unneeded - the layout is still responsive.
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 technically looks better too. It's interesting that without this property, the underlying elements occupy their own minimal space, while if min-width of the parent is specified, they're squeezed inside the parent.
Here's a comparison:
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.
Idk why 🤷♂️