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

Update app with new branding #2576

Merged
merged 3 commits into from
Apr 27, 2021
Merged

Update app with new branding #2576

merged 3 commits into from
Apr 27, 2021

Conversation

shawnborton
Copy link
Contributor

@shawnborton shawnborton commented Apr 26, 2021

Details

This updates the app with our new logo and colors.

Fixed Issues

N/A

Tests / QA Steps

  1. Go to the Expensify.cash sign in page and make sure the screenshot, logo, and colors are updated on all platforms
  2. On web, make sure the favicon is also updated to the new logo
  3. Make sure the app icon is updated to use the new logo too.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image

image

image

image

Mobile Web

image

Desktop

image

image

iOS

image

image

image

image

Android

@shawnborton shawnborton requested a review from a team as a code owner April 26, 2021 17:31
@shawnborton shawnborton self-assigned this Apr 26, 2021
@MelvinBot MelvinBot requested review from Dal-Papa and removed request for a team April 26, 2021 17:32
@shawnborton
Copy link
Contributor Author

Just need to test this out on Android next, but having trouble building the app.

@marcaaron marcaaron removed their request for review April 26, 2021 18:03
@roryabraham
Copy link
Contributor

Tested on Android for ya, and it's looking pretty good. (splash screen also looked correct to me).

image
image

However, the default avatar images look like they come from cloudfront, and I'm not seeing those updated when testing locally...
image

Anyways, the small number of code changes look fine.

@shawnborton
Copy link
Contributor Author

Thanks for helping out with that! I think this is ready for full review and merge then.

@roryabraham
Copy link
Contributor

How did you get the screenshots locally with the updated avatars that come from cloudfront?

@shawnborton
Copy link
Contributor Author

shawnborton commented Apr 26, 2021

Ah, those new avatars are coming from Web-Static changes that we'll deploy alongside all of the app updates.

Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

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

Okay, well I think this PR is working as expected 👍

@shawnborton
Copy link
Contributor Author

@Dal-Papa all you for the final approval!

Copy link

@Dal-Papa Dal-Papa left a comment

Choose a reason for hiding this comment

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

😍

@Dal-Papa Dal-Papa merged commit 5ec3b3f into main Apr 27, 2021
@Dal-Papa Dal-Papa deleted the shawn-rebrand branch April 27, 2021 08:41
@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.31-4🚀

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

@roryabraham
Copy link
Contributor

Looks like iOS deploy failed 😕

@roryabraham
Copy link
Contributor

image

@shawnborton This is now a deploy blocker, do you have a quick fix or should I revert?

@shawnborton
Copy link
Contributor Author

Yeah, I can provide the missing icon if that helps?

@roryabraham
Copy link
Contributor

Yeah, I think we need to provide the missing icon and somehow link it in the PLists. Maybe following this article:

https://medium.com/@this.shoaib/missing-required-icon-file-1893c0bedaaf

@shawnborton
Copy link
Contributor Author

Ah, I see exactly what I did wrong. I made icons for iPad at 72x72 instead of 76x76. So should I just send a separate PR that has the three new icons with no QA, etc? Or what is the best way to proceed? Alternatively, I can just supply the new icons to you if that is easier.

@roryabraham
Copy link
Contributor

Feel free to submit a new PR, this error is still a bit of a mystery to me

@roryabraham
Copy link
Contributor

It looks like it wants 152x152 for iPad

@shawnborton
Copy link
Contributor Author

Here is the follow up PR: #2591

Let me know what I need to do for testing steps, etc.

@roryabraham
Copy link
Contributor

Okay, btw @shawnborton I'm going to mark this as No QA, but really it should be internalQA. We just don't exactly have internal QA process for E.cash yet (we weren't sure we would ever actually need one). We'll just have to keep an eye out on staging and make sure it looks good.

@shawnborton
Copy link
Contributor Author

Sounds good, thanks for helping me out with this one!

@OSBotify
Copy link
Contributor

OSBotify commented May 8, 2021

🚀 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.

4 participants