zulip / zulip-mobile

Zulip mobile apps for Android and iOS.
https://zulip.com/apps/
Apache License 2.0
1.28k stars 643 forks source link

Use "list" icon for all-messages, matching web's new icon there #5303

Open gnprice opened 2 years ago

gnprice commented 2 years ago

We currently use a "globe" icon for the button on the main screen that takes you to the all-messages view. This has never been a great icon to use there, but we didn't feel we had a better one -- the web app used a "home" icon, and that seemed like it would put too much emphasis on that view. (Unlike on web, the all-messages view has never been the main view that we present.)

But now web uses a "list" icon: image

That seems perfectly reasonable, and it's helpful to have the same icon on web as on mobile. So let's switch to that one.

(From chat thread here.)

Vatsalsoni13 commented 2 years ago

Can I work on this?

wilsonfurtado2000 commented 2 years ago

I have created a PR please look into it

avdhendra commented 2 years ago

i can change that icon can you assign me on this

dootMaster commented 2 years ago

Hi, I'd like to help out on this issue.

alya commented 2 years ago

@dootMaster great! Please post a comment describing your proposed approach when you're ready. We can assign the issue to you once you have a rough plan. Thanks!

dootMaster commented 2 years ago

@alya

The goal is to change the icon of mobile Zulip's All Messages tab to be congruent with the icon that currently exists in the browser Zulip app.

Rough plan:

First, I will read the docs: Contributing, and Style Guide.

  1. Change the string property name passed in the TopTabButton component such that when it is consumed by the Icon function in Icons.js, an icon that is congruent to the "list" icon that currently exists on the browser Zulip app is returned.
  2. Run unit tests.
  3. Submit a PR and submit to code review stream on chat.zulip.org.
chrisbobbe commented 2 years ago

Sounds good, thanks, @dootMaster! I've just assigned this issue to you.

dootMaster commented 2 years ago

I've created a draft pull request. I'll also post this in the code review stream on the chat.

edit: tag for visibility @chrisbobbe

aritroCoder commented 1 year ago

I want to work on this issue, please assign it to me

chrisbobbe commented 1 year ago

Sure, I see the current PR #5471 has been awaiting a revision from the author for some time, but it looks like we have a proposed implementation; see https://github.com/zulip/zulip-mobile/pull/5471#issuecomment-1230861599. You can try doing that.

dootMaster commented 1 year ago

Hi, things got a little busy for me. The revision is done; I've created a PR awaiting review.

alya commented 1 year ago

@dootMaster What's the right PR to look at? #5458? (I guess GitHub incorrectly show it as merged?)

dootMaster commented 1 year ago

@alya #M5532. Yeah, GitHub is incorrectly showing the other as merged.

entrymaster commented 1 year ago

I checked the app, we still have this issue. I want to work on this.

alya commented 1 year ago

I think the next step here is for maintainers to re-review #5532.

ashutosh887 commented 1 year ago

@alya Is it resolved?