Tweak camera flash control to make it more obvious#53554
Tweak camera flash control to make it more obvious#53554mountiny merged 5 commits intoExpensify:mainfrom
Conversation
Signed-off-by: GitHub <noreply@github.com>
|
@allgandalf Please 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] |
|
@mountiny , should we trigger adhoc for this one ? changes should work but can't test the functionality on simulators/emulators :)) |
Signed-off-by: GitHub <noreply@github.com>
|
🚧 @mountiny has triggered a test build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@dubielzyk-expensify can you please provide that svg file , thanks |
|
All of our svg icons can be styled via fill color though, I don't think we've ever provided different light/dark mode icons. Can you take a look at the implementation? We have icons all throughout the app that work this way just fine. |
|
@twilight2294 please check too! |
|
@shawnborton @dubielzyk-expensify can you please share that icon file again? i tired almost all the theme colours present, the colour is not changing to light :)) |
|
I checked more and it is this part in the svg file: Which is causing the problem. if i remove this part then thing work fine @shawnborton @dubielzyk-expensify @mountiny @allgandalf : Screen.Recording.2024-12-05.at.2.24.26.PM.movWhat should i do now? remove that part from svg myself or wait for correct svg file ? |
|
umm, makes sense to me, thanks for investigating @twilight2294 👍 |
Lets wait for new file I would say |
|
Hmm that's odd. Let's try this one? |
|
New image works, @mountiny @dannymcclain @allgandalf can you apply the |
|
🚧 @dannymcclain has triggered a test build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
Reviewer Checklist
Screenshots/VideosAndroid: NativeXRecorder_05122024_211356.mp4Android: mWeb ChromeXRecorder_05122024_212240.mp4iOS: NativeScreenRecording_12-05-2024.21-01-18_1.MP4iOS: mWeb SafariScreenRecording_12-05-2024.21-04-06_1.MP4 |
|
Videos are looking good to me 👍 |
|
Agree, looks good to me too 👍 |
|
There's some micromovement going on with the icon: It's because there's not a perfect overlap in the icons. Turns out the one in Figma is different from the one in our repo: Let's update the regular |
|
@dubielzyk-expensify the file you just sent also has the style : Can you send the updated file please ?, the current file will cause the icon to be dark |
|
Let's see if this works: |
|
I have updated it accordingly : Screen.Recording.2024-12-06.at.2.50.19.PM.mov |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Performance Comparison Report 📊Significant Changes To Duration
Show details
Meaningless Changes To DurationShow entries
Show details
|
|
@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker. |
|
I don't think so changing icon would cause performance regression, maybe false positive ? |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.0.73-0 🚀
|
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.0.73-8 🚀
|





Explanation of Change
Fixed Issues
$ #53422
PROPOSAL: #53422 (comment)
Tests
Test only for small screen platform (Cannot test on devices with no flash present):
Offline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop