Page MenuHomePhabricator
Differential D9553

[lib/native] introduce useUpdateThemePreference
ClosedPublic
Actions

Authored by ginsu on Oct 20 2023, 12:34 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 20, 7:18 AM
Unknown Object (File)
Sat, Nov 30, 4:38 PM
Unknown Object (File)
Mon, Nov 25, 7:57 AM
Unknown Object (File)
Nov 22 2024, 3:41 AM
Unknown Object (File)
Nov 22 2024, 2:14 AM
Unknown Object (File)
Nov 21 2024, 9:20 PM
Unknown Object (File)
Nov 14 2024, 11:08 PM
Unknown Object (File)
Nov 14 2024, 11:25 AM
View All 82 Files
Subscribers
ashoat
tomek
will

Details

Reviewers
atul
inka
rohan
Commits
rCOMM4d90492abaeb: [lib/native] introduce useUpdateThemePreference
Summary

We are going to want this exact logic in our web AppearanceChangeModal. Rather than copy + paste this logic, this diff preemptively factors out this logic into a seperate hook that can be conusmed by each platform

Part of https://linear.app/comm/issue/ENG-4958/update-theme-based-on-the-active-theme-redux-state

Depends on D9552

Test Plan

flow + confirmed that there were no regressions with updating the app theme

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.Oct 20 2023, 12:34 AM
Herald added subscribers: will, tomek, ashoat. · View Herald TranscriptOct 20 2023, 12:34 AM
ginsu added inline comments.Oct 20 2023, 12:36 AM
lib/hooks/theme.js
53–56 ↗(On Diff #32246)

I thought this was more readable than

const theme =
  themePreference === 'system'
    ? this.props.globalThemeInfo.systemTheme
    : themePreference;

happy to change it back to this if people prefer this

ginsu added reviewers: atul, inka, rohan.Oct 20 2023, 12:36 AM
ginsu added a child revision: D9554: [web] update AppearanceChangeModal buttons to use redux state.Oct 20 2023, 12:47 AM
ginsu edited the summary of this revision. (Show Details)
ginsu added a parent revision: D9552: [web] introduce redux persist migration for globalThemeInfo.
Harbormaster completed remote builds in B23396: Diff 32246.Oct 20 2023, 12:51 AM
ginsu requested review of this revision.Oct 20 2023, 12:51 AM
atul accepted this revision.Oct 20 2023, 10:47 AM
atul added inline comments.
native/profile/appearance-preferences.react.js
63 ↗(On Diff #32246)

Almost got me, but looks like this is class component

This revision is now accepted and ready to land.Oct 20 2023, 10:47 AM
ginsu updated this revision to Diff 32322.Oct 23 2023, 11:03 AM

rebase before landing

This revision was landed with ongoing or failed builds.Oct 23 2023, 11:15 AM
Closed by commit rCOMM4d90492abaeb: [lib/native] introduce useUpdateThemePreference (authored by ginsu). · Explain Why
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMM4d90492abaeb: [lib/native] introduce useUpdateThemePreference.
Harbormaster completed remote builds in B23430: Diff 32322.Oct 23 2023, 11:22 AM

Revision Contents
Changeset List

PathSize
lib/
hooks/
37 lines
native/
profile/
30 lines

Diff 32322

View Options

lib/hooks/theme.js

Loading...
View Options

native/profile/appearance-preferences.react.js

Loading...