Fix searchbar overflowing for mobile #2009
Conversation
There was a problem hiding this comment.
Thanks @domlimm 👍
You may also want to add a media query for tablets (>767px and <879px). Currently the search bar is still overflowing for longer placeholders for the above range.
| .form-control { | ||
| min-width: 8em; | ||
| max-width: 16em; /* twice of min-width, to accommodate a range of lengths */ | ||
| } |
There was a problem hiding this comment.
Some styling issue here causing the linting to fail. You may want to run npm run lintfix to help lint the code next time :)
There was a problem hiding this comment.
Yes, that's right! Got to make running npm run lint a habit. Didn't realise the styles are using 4 spaces instead of 2. Thanks for the advice!!
|
Hey @jonahtanjz! Thanks for reviewing! And yes, slipped my mind to test for tablets 😅 Will add it in now! 👍🏻 |
|
@jonahtanjz Hey Jonah! Sorry that took awhile, was trying out what works best. FYI, if it's still overflowing when you are testing, you could refresh the page and it should work accordingly. Thank you, appreciate it! 😄 |
|
@tlylt Yes, same behaviour at my end. Attached a demo below. By the way, the page flashes when it reloaded with F5. Maybe might have to tweak the range. demo.mp4From w3schools: /* Extra small devices (phones, 600px and down) */
@media only screen and (max-width: 600px) {...}
/* Small devices (portrait tablets and large phones, 600px and up) */
@media only screen and (min-width: 600px) {...}
/* Medium devices (landscape tablets, 768px and up) */
@media only screen and (min-width: 768px) {...}
/* Large devices (laptops/desktops, 992px and up) */
@media only screen and (min-width: 992px) {...}
/* Extra large devices (large laptops and desktops, 1200px and up) */
@media only screen and (min-width: 1200px) {...} |
|
I think this has more to do with the issue #1699. The width of the search bar will have to vary depending on the number of items in the navbar menu. There are a couple of solutions discussed here #1760 (review) if you are interested. This can be done in another PR :) |
008afe9 to
a39a92d
Compare
| } | ||
|
|
||
| /* For general tablets in portrait e.g. iPad */ | ||
| @media screen and (min-width: 768px) and (max-width: 878px) and (orientation: portrait) { |
There was a problem hiding this comment.
How about combining this with the above query since they have the same property?
@media screen and (max-width: 767px), screen and (min-width: 768px) and (max-width: 878px) and (orientation: portrait) {
.form-control {
min-width: 8em;
max-width: 16em; /* twice of min-width, to accommodate a range of lengths */
}
}There was a problem hiding this comment.
Thanks Jonah! Will make the change.
What is the purpose of this pull request?
Overview of changes:
Fixes #2002.
Added a media query to handle for mobile screens (not too familiar but
max-width: 768pxseems to suffice).In the media query, we set the
min-widthandmax-widthto follow the styling of the current prod's search bar.Also, with reference to PR #1959 to not interfere with the implementation of it, and follow it by giving the search bar a
min-widthandmax-width.Anything you'd like to highlight/discuss:
NIL
Testing instructions:
Served locally.
Proposed commit message: (wrap lines at 72 characters)
Fix searchbar overflow for mobile screens
Checklist: ☑️