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

[$1000] Update Concierge icon #12264

Closed
shawnborton opened this issue Oct 28, 2022 · 57 comments
Closed

[$1000] Update Concierge icon #12264

shawnborton opened this issue Oct 28, 2022 · 57 comments
Assignees
Labels
Daily KSv2 Design Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review

Comments

@shawnborton
Copy link
Contributor

shawnborton commented Oct 28, 2022

Note: This issue is part of the greater Implement Our New Branding in NewDot project.

What is it:
Replace the Concierge bell icon with a new icon

image

Does it require a mini design doc:
No

Design finished:
Maybe?

cc @GabiHExpensify

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0115bd027f7f77fe7b
  • Upwork Job ID: 1600615237124608000
@shawnborton shawnborton self-assigned this Oct 28, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 28, 2022

Auto-assign attempt failed, all eligible assignees are OOO.

@mountiny
Copy link
Contributor

mountiny commented Nov 2, 2022

For completeness, we would like to show this new icon for Concierge only in NewDot and leave the current Concierge avatar in other platforms.

@NikkiWines
Copy link
Contributor

@shawnborton could you link/post the svg format of the new concierge icon?

@mountiny mountiny added Engineering NewFeature Something to build that is a new item. labels Nov 2, 2022
@shawnborton
Copy link
Contributor Author

Can do! Is it better to have it with or without the roundedness?

@NikkiWines
Copy link
Contributor

Hmmm with roundedness for now, I think! That might change with the broader default icon application though 🤔

@NikkiWines
Copy link
Contributor

Actually, @shawnborton I think I can just use the one you sent earlier if that's fine by you. We'd need to add it to web-static

Large Concierge Icon (png)

concierge2022

@NikkiWines
Copy link
Contributor

And then for implementation, I think it's fairly simple.

For now, we'd just make a new const CONCIERGE_ICON_URL: ${CLOUDFRONT_URL}/images/concierge_2022.png in CONST.js
and then we can just add the following few lines in this _.each statement:

if (login == CONST.EMAIL.CONCIERGE) {
    personalDetail.avatar = CONCIERGE_ICON_URL
}

That way we'll only update the avatar in NewDot. Once we consolidate the icons between oldDot and newDot we can remove that code and simply update the avatar stored in concierge@expensify.com's expensify_personalDetails NVP.

@shawnborton
Copy link
Contributor Author

shawnborton commented Nov 2, 2022

Let's use an .svg instead to it retains the crisp quality we want.

Here you go:
ExpensifyConciergeIcon 2.svg.zip
ExpensifyConciergeIcon

@shawnborton
Copy link
Contributor Author

(I uploaded a new version above to fix the weird star

@NikkiWines
Copy link
Contributor

NikkiWines commented Nov 2, 2022

Oh and if we also want to update the Help Site we'd just need to update this icon.

It looks like it's not quite the same as the current concierge icon (more blue padding) so not sure if we'd want an alternative new icon as well

@mountiny
Copy link
Contributor

mountiny commented Nov 2, 2022

Nice solution!

@JmillsExpensify
Copy link

Wow, nice progress on this in such a short amount of time @NikkiWines. This is super exciting!

@NikkiWines
Copy link
Contributor

Thanks everyone 😊

@shawnborton did we want to also update the Help Site or shall we leave that concierge icon as is?

@NikkiWines
Copy link
Contributor

Also, it looks like we're unable to render svgs for iOS and Android apps (works fine on web, desktop, and mweb) so we'll need to use the png version of the icon. Is the one from this comment ok?

@NikkiWines NikkiWines mentioned this issue Nov 3, 2022
95 tasks
@shawnborton
Copy link
Contributor Author

We use a lot of svgs for both iOS and Android. All of our one-color icons are svgs, as well as things like room icons and default profile pictures.

@shawnborton
Copy link
Contributor Author

As for the help site, let's leave that alone for now and tackle that one separate from these NewDot updates.

@JmillsExpensify
Copy link

Out of curiosity, why not update the Concierge icon on the help site as well?

@JmillsExpensify
Copy link

Only reason I ask is that this new help site is exclusively focused on NewDot, where we're updating Concierge.

@shawnborton
Copy link
Contributor Author

Ah good point, is the help site baked into the App repo?

@mountiny
Copy link
Contributor

mountiny commented Nov 3, 2022

is the help site baked into the App repo?

Yes

@NikkiWines
Copy link
Contributor

NikkiWines commented Nov 3, 2022

We use a lot of svgs for both iOS and Android. All of our one-color icons are svgs, as well as things like room icons and default profile pictures.

Sorry, I was unclear here. There are two problems with svgs. For the ios app and android app specifically there's a problem using svgs from a remote resource (like cloudfront, if we pull the icon from https://d2k5nsl2zxldvw.cloudfront.net/images/icons/concierge_2022.svg). If we change that to an png, it works across all platforms.

If we keep the image local to the /assets/ folder (which we definitely can do) that works better but there's a small problem with displaying svgs in full format (across all platforms), like so:

Screen.Recording.2022-11-03.at.15.46.05.mov

Though, this same issue exists for the fallback avatars, so maybe it's ok?

Screen.Recording.2022-11-03.at.16.01.20.mov

@shawnborton
Copy link
Contributor Author

Oh wow, do you know why we can't show the full svg image in the attachment modal? Maybe that's a separate problem we can fix?

@NikkiWines
Copy link
Contributor

I believe it's a limitation of react native, we might be able to get around it by using SvgComponent from the react-native-svg library, but I personally think that's a separate issue from this one.

As an aside, the current icon we're using for concierge is also a png.
image

@JmillsExpensify
Copy link

Circling back on the SVG convo above, we're going to work on that on in #12658.

@mananjadhav
Copy link
Collaborator

@JmillsExpensify @NikkiWines Can anyone of you help me the Upwork payment here? It's hit production and completed the 7 days regression period.

@JmillsExpensify JmillsExpensify self-assigned this Dec 7, 2022
@JmillsExpensify JmillsExpensify added the External Added to denote the issue can be worked on by a contributor label Dec 7, 2022
@melvin-bot

This comment was marked as off-topic.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Monthly KSv2 labels Dec 7, 2022
@melvin-bot melvin-bot bot changed the title Update Concierge icon [$1000] Update Concierge icon Dec 7, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 7, 2022

Job added to Upwork: https://www.upwork.com/jobs/~0115bd027f7f77fe7b

@melvin-bot

This comment was marked as off-topic.

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 7, 2022
@melvin-bot

This comment was marked as off-topic.

@JmillsExpensify
Copy link

Sure thing. Upwork job is immediately above.

@JmillsExpensify JmillsExpensify removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 7, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 7, 2022

📣 @mananjadhav You have been assigned to this job by @JmillsExpensify!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@JmillsExpensify
Copy link

@NikkiWines Not sure why this wasn't closed but I'm going to do so once @mananjadhav is paid out. Cool?

@NikkiWines
Copy link
Contributor

Yeah not sure why it didn't close, since #12425 references this issue. But yes that sounds good 👍

@NikkiWines
Copy link
Contributor

Also weird that this issue suddenly showed up here...

@JmillsExpensify
Copy link

Hmm, yeah that's super odd.

@JmillsExpensify
Copy link

Also invitation sent for @mananjadhav. I suspect he'll accept shortly and we can close this out.

@mananjadhav
Copy link
Collaborator

Accepted @JmillsExpensify

@JmillsExpensify
Copy link

Thanks, I just sent you the offer. Accept there and I'll pay you out now.

@JmillsExpensify
Copy link

Alright, all paid out! Thanks ya'll, I'm closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Design Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

7 participants