-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Better tablet layout #7656
base: main
Are you sure you want to change the base?
Better tablet layout #7656
Conversation
|
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.
Fixed a few more spots:
|
This works really well on my iPad (9th gen), nice work @mozzius! Having the search bar, feed list and Trending visible is a massive improvement, thanks! 🙏 I realise it would be out-of-scope for this PR, but just wondering, how technically feasible would it be in the future to move the PWI language selector to the right nav when in tablet layout? The app language is usually autodetected of course so it's not a big deal, but in case there is a desire to change it this would let tablet users do so when they're viewing the PWI (and without changing the zoom setting) and fix #3578. |
Great idea @surfdude29! added! |
Oh nice, thanks! :) |
One other small point to mention: should Trending be showing up in the PWI? (Fwiw it should imho.) Sometimes it shows up for me, sometimes not, so reloading the page can occasionally cause it to appear or disappear. |
It should be showing up, yes. Hailey fixed a bug yesterday which was causing ~1% of requests to fail, so hopefully it should be a little more reliable - however, there's some sort of bug with en-GB which I'm still trying to get to the bottom of |
Ah ok, maybe I'm running into the en-GB bug on the preview branch. Edit: Sorry to bother you with yet another thing, but I noticed (on main.bsky.dev) that the X button to close Trending is visible in the PWI. I'm not sure that this is ideal because tapping it brings up this dialog, which doesn't exactly make sense if you're not logged in: Is it worth creating an issue for that? |
Sure, I guess that is broken |
10d159d
to
1dd65ab
Compare
Fixes #3578
Adds new breakpoints for web:
I also moved the left nav on the tablet breakpoint to sit next to the content, rather than being stuck to the far left edge
1075px to 1200px
800px to 1075px
Test plan
Check all the screens at the new breakpoints. I have tested:
And make sure there's no regressions on the mobile/desktop breakpoints