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

homescreen_tabs: Change "globe" icon to "align-left" icon. #5456

Closed
wants to merge 1 commit into from
Closed

homescreen_tabs: Change "globe" icon to "align-left" icon. #5456

wants to merge 1 commit into from

Conversation

dootMaster
Copy link

@dootMaster dootMaster commented Aug 3, 2022

This change was made to make the All-Messages icon in mobile Zulip congruent to the one found in browser Zulip.

This swap was performed by changing the name property of the first TopTabButton component found in HomeScreen.js. This property was changed from globe to align-left. It is worthwhile to note that this property is a string restricted by Flow's literal union type. A list of all valid strings for this property can be found in FeatherGlyphs type at the top of Icons.js.

Screen Shot 2022-07-30 at 5 41 14 PM

Screen Shot 2022-07-30 at 5 41 49 PM

Both previous and updated icons are a part of the Feather icons collection inside the React-Native-Vector-Icons. While it seems Zulip mobile uses Feather icons almost exclusively, if it is decided an icon from a different collection is more appropriate, changes must be made to the Icon component that consumes the name property. In its current state, the component contains helper functions (see: fixIconType) that are configured to build only from the Feather collection.

This commit has passed the included unit tests.
Screen Shot 2022-07-30 at 5 39 51 PM

Fixes: #5303.

This change was made to make the All-Messages icon in mobile Zulip congruent to the one found in browser Zulip.

This change was performed by changing the "name" property of the first TopTabButton component found in HomeScreen.js. This property was changed from "globe" to "align-left".

It is important to note that this property is a string restricted by Flow's literal union type. A list of all valid strings for this property can be found in "FeatherGlyphs" type at the top of Icons.js.

This commit has passed all included tests.

Fixes: #5303.
@dootMaster
Copy link
Author

There are improvements that @chrisbobbe mentioned in another PR that aren't explicitly stated in the docs.

I'm closing this PR to fix it up further.

@dootMaster dootMaster closed this Aug 4, 2022
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.

Use "list" icon for all-messages, matching web's new icon there
1 participant