Skip to content

ui: Sweep through and fix safe-area bugs #413

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 27, 2023
Merged

ui: Sweep through and fix safe-area bugs #413

merged 4 commits into from
Nov 27, 2023

Conversation

chrisbobbe
Copy link
Collaborator

This fixes all the safe-area bugs I noticed in a sweep through the UI, all in the style of option 2 described here: #339 (comment)

Screenshots coming soon.

Fixes: #339

@chrisbobbe
Copy link
Collaborator Author

chrisbobbe commented Nov 22, 2023 •
edited
Loading

cc @terpimost @alya

"Direct messages" before/after:

image image

"Inbox" before/after:

image image

"Streams" before/after:

image image

@chrisbobbe
Copy link
Collaborator Author

@sirpengi, would you mind checking this commit to see if I'm missing something:

c401f93 subscription_list [nfc]: Remove unneeded [Center]

@alya
Copy link
Collaborator

alya commented Nov 23, 2023

Cool, looks good to me!

@chrisbobbe @gnprice
recent_dm_conversations: Handle insets with a SafeArea
0eac54f 
Fixes: zulip#339
@chrisbobbe @gnprice
inbox: Handle insets with a SafeArea
7583522
@chrisbobbe @gnprice
subscription_list [nfc]: Remove unneeded [Center]
0618adc 
I think this isn't necessary; the child will expand out as far as
the parent constraints allow, so a centered alignment isn't
different from any other kind of alignment.
@chrisbobbe @gnprice
subscription_list: Handle insets with a SafeArea
93cefca
@gnprice
Copy link
Member

gnprice commented Nov 27, 2023

Thanks for these fixes! Looks good; merging.

@gnprice gnprice merged commit 93cefca into zulip:main Nov 27, 2023
@terpimost
Copy link

I'm not sure why do we bother with horizontal layout at all, but it is great quality if we do such things.

I personally think that we shouldn't bother with horizontal layout at all, because it is more annoying if the vertically oriented app is trying to flip itself if I'm trying to read zulip while laying on a bed. Yeah... there is OS level setting, but I don't think we should spend our resources in attempt to support it. It is just too much work for design and testing...

@gnprice
Copy link
Member

gnprice commented Nov 27, 2023

I'm not sure why do we bother with horizontal layout at all, but it is great quality if we do such things.

Thanks for the feedback!

I personally think that we shouldn't bother with horizontal layout at all,

I don't have a strong view on that myself. This is a good discussion to have in chat, in the #mobile stream where we can get feedback from more of the community. I've started a thread there: https://chat.zulip.org/#narrow/stream/48-mobile/topic/landscape.20mode/near/1689267

@gnprice gnprice mentioned this pull request Nov 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnprice gnprice Awaiting requested review from gnprice

@sirpengi sirpengi Awaiting requested review from sirpengi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RecentDmConversationsPage: Breaks with nonzero horizontal device insets
4 participants
@chrisbobbe @alya @gnprice @terpimost