Skip to content
This repository was archived by the owner on Oct 11, 2022. It is now read-only.

Conversation

@brianlovin
Copy link
Contributor

Status

  • WIP
  • Ready for review
  • Needs testing

Deploy after merge (delete what needn't be deployed)

  • mobile

Updates tab bar icons to fit HIG spec, fixes send icon. h/t @uberbryn :)

Copy link
Contributor

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mxstbr mxstbr merged commit 63697c2 into alpha Jun 12, 2018
@mxstbr mxstbr deleted the fix-mobile-icons branch June 12, 2018 07:35
@mxstbr
Copy link
Contributor

mxstbr commented Jun 12, 2018

Actually, now that I look at 'em these icons look really weird and don't really fit together:

screen shot 2018-06-12 at 10 30 09

The search icon feels out of place since it doesn't have any fill, and the profile icon feels really bold because it has the wrapper around it. Why did we change them to be the filled versions? If they have to be filled, can we at least make it so all of them have the squircle wrapper like the profile one?

@superbryntendo
Copy link
Contributor

  1. Apple's standard wants them as filled icons. I personally prefer the stroked icons as well, though.
  2. I think they look pretty consistent except that the Messages icon feels off-center vertically and yes - of course the Search icon looks completely out of place, but that should go away asap to be replaced with Explore.

Also, I think we should move Search to the end of the nav bar in the meantime.

@mxstbr
Copy link
Contributor

mxstbr commented Jun 12, 2018

Let's just go with the stroked ones then?

@brianlovin
Copy link
Contributor Author

cc @uberbryn just ping me when you want to swap anything out; assuming you have mobile running locally so you can see it on-device, but lmk if i can help there

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants