[native] change color theme of the emoji keyboard based on the app theme
ClosedPublic
Actions

Authored by ginsu on Jun 23 2023, 12:26 PM.

Details

Reviewers

atul
kamil

Commits

rCOMM50c0adf387b7: [native] change color theme of the emoji keyboard based on the app theme

Summary

This diff adds a color theme to the emoji keyboard based on the theme of the app. I tried to reuse as many of the preexisting color variables as I could; however, there were a few theme properties of the emoji keyboard where I needed to create a new color variable

Here are some docs that I used to help me implement this:
https://thewidlarzgroup.github.io/rn-emoji-keyboard/docs/api/theme

Linear Task: https://linear.app/comm/issue/ENG-3793/mobile-emoji-keyboard-darklight-mode-theme

Depends on D8309

Test Plan

Here are the screenshots of what the emoji keyboard looks like and I spoke with @ted and got his stamp of approval

Light mode:

Screenshot 2023-06-23 at 12.21.20 PM.png (1×1 px, 649 KB)

Dark mode:

Screenshot 2023-06-23 at 12.20.56 PM.png (1×1 px, 637 KB)

Diff Detail

Repository

rCOMM Comm

Branch

eng-3790 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Jun 23 2023, 12:26 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJun 23 2023, 12:26 PM

ginsu edited the summary of this revision. (Show Details)Jun 23 2023, 12:30 PM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: atul, kamil.

ginsu added a parent revision: D8309: [native] style emoji keyboard.

ginsu attached a referenced file: F604244: Screenshot 2023-06-23 at 12.20.56 PM.png. (Show Details)

ginsu attached a referenced file: F604243: Screenshot 2023-06-23 at 12.21.20 PM.png. (Show Details)

fix emojiKeyboardContainerActive variable name

ginsu published this revision for review.Jun 23 2023, 12:34 PM

Harbormaster completed remote builds in B20478: Diff 28060.Jun 23 2023, 12:44 PM

Harbormaster completed remote builds in B20479: Diff 28062.Jun 23 2023, 12:51 PM

kamil accepted this revision.Jun 26 2023, 1:32 AM

This revision is now accepted and ready to land.Jun 26 2023, 1:32 AM

rebase before landing

This revision was landed with ongoing or failed builds.Jun 26 2023, 10:29 AM

Closed by commit rCOMM50c0adf387b7: [native] change color theme of the emoji keyboard based on the app theme (authored by ginsu). · Explain Why

This revision was automatically updated to reflect the committed changes.

ginsu added a commit: rCOMM50c0adf387b7: [native] change color theme of the emoji keyboard based on the app theme.

Harbormaster completed remote builds in B20508: Diff 28117.Jun 26 2023, 10:43 AM

ashoat added inline comments.Jun 28 2023, 4:02 PM

native/components/emoji-keyboard.react.js
98–117 ↗	(On Diff #28119)	We should not be mixing color families like this! I definitely would've requested changes if I saw this in time. @ginsu, can you create a task and prioritize addressing this? We need to stop taking this laissez-faire approach with color families. It's the responsibility of everybody on the team to take colors / theming seriously if we want to get out of the pit we've dug ourselves into.

ashoat added inline comments.Jun 28 2023, 4:04 PM

native/themes/colors.js
144–146 ↗	(On Diff #28119)	Do we really need one-offs here? Why can't we find an existing color family that works with our designs? We need to push the team (@tedchang in particular) to use existing families (and update them as necessary) instead of hacking around inconsistencies like this. When we take this sort of approach of picking colors on a per-screen basis, we fail to maintain stylistic / design consistency throughout the app
144–146 ↗	(On Diff #28119)	Meant to tag @ted