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

Followups from #4679 #4768

Merged
merged 5 commits into from
Jun 9, 2021
Merged

Followups from #4679 #4768

merged 5 commits into from
Jun 9, 2021

Conversation

WesleyAC
Copy link
Contributor

@WesleyAC WesleyAC commented May 28, 2021

Fixes: #4766

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Jun 3, 2021

I think this needs a rebase past #4750 which added a openLinkWithUserPreference call; would you mind doing that?

@WesleyAC
Copy link
Contributor Author

WesleyAC commented Jun 3, 2021

Ah, yep! Thanks for catching that, done.

This makes the OptionRow always the same height, regardless of whether
there's an icon or not. As far as I can tell, this doesn't change any of
the current uses of OptionRow (since almost all of them currently use
icons).

The value of 56 was discovered by inspecting OptionRows with icons -
since RNVI uses font-size instead of width/height, there isn't a nice
way to get the height of an icon from its size.

This is in preparation for removing some icons from the settings screen.
Since the "Chrome" icon for the in-app browser setting was confusing,
remove that icon. And since it looks strange to have icons on some but
not all settings, this also removes the icon for "Night Mode", leaving
us with icons only for groups of settings, not individual settings.
@gnprice
Copy link
Member

gnprice commented Jun 9, 2021

Looks good, thanks! Merged.

@gnprice gnprice merged commit c897133 into zulip:master Jun 9, 2021
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.

Followups on open-links-where setting, #4679
3 participants