-
Notifications
You must be signed in to change notification settings - Fork 3k
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 NewDot Logos Nov 2022 #12608
Update NewDot Logos Nov 2022 #12608
Conversation
@parasharrajat @puneetlath One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Please merge the main. |
Can we remove the HOLD from the title of this is ready? |
Ready for final review/merge! |
Looks good to me! The splash screen logo on iOS feels slightly too small, any idea why that might be? |
For Splash screens, On Android =>Please update the colors in this file main/android/app/src/main/res/values/styles.xml as well. This file is used when the app boots up and the splash screen is shown. It uses colors from main/android/app/src/main/res/values/colors.xml iOS =>I have never touched it but it seems that we need to
If we are not interested to update that in this PR. Let's first create an issue for it before we merged this. |
I tried that method to change the color but it doesn't seem to work. Technically RGBA(1,1,1,1) is black so I'm actually not sure if that is where we would set it. I think we can make an issue for the mobile splash screens. The android splash can be updated easily- but I think we should wait to update both together later. |
@marcaaron since you set up the splash screens in this PR, any idea how to change IOS background color? |
The package we are using (I think) is this one -> https://github.com/zoontek/react-native-bootsplash Looks like there are some mentions of background color in there. But it also looks like we made a ton of changes to the splash screen in this PR and embedded that package 🤔 I'm a bit behind on how it works now. But would guess that we need to define the color as a global in Android maybe here: App/android/app/src/main/res/values/colors.xml Lines 2 to 3 in b1e517d
Used here: App/android/app/src/main/res/drawable/bootsplash.xml Lines 4 to 5 in b1e517d
And for iOS maybe that happens here in the App/ios/NewExpensify/BootSplash.storyboard Lines 27 to 28 in b1e517d
Gonna tag in @NikkiWines and @zoontek to see if they are available to help advise further. |
Okay I figured it out, it's the inverse color. I used https://wtools.io/invert-color-code-online to get rgb(249, 228, 246) |
Nice, that will do - thanks for digging into that one. |
I changed in colors PR, logos can be merged once someone submits the PR review checklist |
Reviewer Checklist
Screenshots/VideosWebweb.movMobile Web - Chromechrome.movMobile Web - Safarisafari.movDesktopdesktop.moviOSios.movAndroidandroid.mov |
🚀 Deployed to staging by @luacmartins in version: 1.2.31-1 🚀
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
The production deploy comment failed for this PR, but this was deployed to production on v1.2.32-2 on Nov 28. |
Details
Update to our New Expensify Logos!

Holding for official color-switch PR, but ready for review.
Fixed Issues
$ #12441
Tests
QA Steps
Same checks as tests section.
PR Author Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
have been tested & I retested again)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Screenshots/Videos
Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android
PR Reviewer Checklist
The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
have been tested & I retested again)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Screenshots
Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android