Skip to content

Use the normal pin icon for the LHN#2686

Merged
MonilBhavsar merged 1 commit into
mainfrom
tgolen-new-pin-icon
May 5, 2021
Merged

Use the normal pin icon for the LHN#2686
MonilBhavsar merged 1 commit into
mainfrom
tgolen-new-pin-icon

Conversation

@tgolen

@tgolen tgolen commented May 4, 2021

Copy link
Copy Markdown
Contributor

cc @shawnborton

Details

Just swapping out the icon and decreasing their size a little bit.

Fixed Issues

Fixes #2666

Tests / QA

  1. Have a draft message on one report, have another report pinned, and another report pinned with a draft
  2. Verify the pin icon in the LHN does not have the circle behind it anymore and the icons are slightly smaller

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image

Mobile Web

image

Desktop

image

iOS

image

Android

image

@tgolen tgolen requested a review from a team May 4, 2021 21:02
@tgolen tgolen self-assigned this May 4, 2021
@MelvinBot MelvinBot requested review from MonilBhavsar and removed request for a team May 4, 2021 21:02
@MonilBhavsar MonilBhavsar merged commit 33adf73 into main May 5, 2021
@MonilBhavsar MonilBhavsar deleted the tgolen-new-pin-icon branch May 5, 2021 10:58
@OSBotify

OSBotify commented May 5, 2021

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging in version: 1.0.37-1🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@kavimuru

kavimuru commented May 5, 2021

Copy link
Copy Markdown

Pen icon not displayed for the draft message in LHN

Expected Result:

Pen for a draft message in the LHN should be present

Actual Result:

Pen for a draft message in the LHN isn't present

Actions Performed:

  1. Go to https://staging.expensify.cash
  2. Log in with any account
  3. Select any user and pinned it
  4. Have another user pinned
  5. Go back to the first user and write the message, but don't send it
  6. Go back to LHN

Platform:

iOS ✔️
mWeb ✔️
Android ✔️

Build:

1.0.38-0

Notes/Images/Video:

20210505_145128.mp4
RPReplay_Final1620242271.mp4

@tgolen

tgolen commented May 5, 2021 via email

Copy link
Copy Markdown
Contributor Author

@isagoico

isagoico commented May 5, 2021

Copy link
Copy Markdown

Oh I see the issue here.

So for the draft icon to appear you have to write something in chat > got to LHN > then navigate to another chat, if I follow these steps the PR is a PASS. If you simply navigate to LHN the conversation is still "open" and draft icon will not appear.

This is the expected behavior at the moment right?

@tgolen

tgolen commented May 6, 2021

Copy link
Copy Markdown
Contributor Author

I think that's what is expected at this point, yeah. The reason being that if you are writing a draft on the currently open chat, there isn't any need to show the draft icon in the LHN.

However, I was confused about this as well when I first saw it and so I think just displaying the draft icon in the LHN (regardless if it is the currently selected chat or not) would be a better user experience.

If you also agree that would be more intuitive, then I think we should move forward with making that change.

@isagoico

isagoico commented May 6, 2021

Copy link
Copy Markdown

I agree that it would be more intuitive for the mobile app. When I tested it on my side it did felt like the draft icon should appear when navigating to the LHN.

@tgolen

tgolen commented May 6, 2021

Copy link
Copy Markdown
Contributor Author

All right, I'll create an issue for it! Thanks.

@tgolen

tgolen commented May 6, 2021

Copy link
Copy Markdown
Contributor Author

Created it over here: https://github.com/Expensify/Expensify/issues/163084

@OSBotify

OSBotify commented May 8, 2021

Copy link
Copy Markdown
Contributor

🚀 Deployed to production in version: 1.0.39-5🚀

platform result
🤖 android 🤖 failure ❌
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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.

LHN - Pin icon seems clickable when it's not

5 participants