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

fix startup nav and dark mode logic #28306

Merged

Conversation

chrisnojima
Copy link
Contributor

@chrisnojima chrisnojima commented Jan 24, 2025

In a recent change we lost the ability to restart from the tab/convo of the last startup. This is because the old method was a little flakey and changing things like the nav or new arch on/off causes this to break sometimes.

We used to have to manage a bunch of state and flags and keys depending on the app state, the 'initialState' we used to track this nav state, and dark mode. Instead we

  1. Handle dark mode differently and simpler
  2. Init the nav after we'd bootstrapped and do it once

This lets us remove a bunch of code (esp dark mode handling) and a lot of convoluted logic

  • ios dev
  • ios prod
  • desktop dev
  • desktop prod
  • android dev
  • android prod

@@ -37,7 +37,8 @@ const ShhIcon = React.memo(function ShhIcon() {
const useMaxWidthStyle = () => {
const {width} = useWindowDimensions()
const hasBadge = useBackBadge() > 0
return React.useMemo(() => ({maxWidth: width - 140 - (hasBadge ? 40 : 0)}), [width, hasBadge])
const w = width - 140 - (hasBadge ? 40 : 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

when starting up already on this view when we init it has some race, so just set the size

@@ -117,6 +117,11 @@ const Icon = React.memo<Props>(
}
}

if (props.skipColor) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

hacky but we want the icon to just use css

@@ -103,36 +103,17 @@ export const useState_ = Z.createZustand<State>((set, get) => {

if (!changed) return

const checkNav = (version: number) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🔥

C.useDarkModeState.getState().dispatch.loadDarkPrefs()
C.useChatState.getState().dispatch.loadStaticConfig()
} finally {
wait(name, version, false)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

make sure we never fail to ref count down

@@ -0,0 +1,314 @@
diff --git a/node_modules/react-native/Libraries/Text/TextInput/Multiline/RCTUITextView.mm b/node_modules/react-native/Libraries/Text/TextInput/Multiline/RCTUITextView.mm
Copy link
Contributor Author

Choose a reason for hiding this comment

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

add a patch to work around dynamic colors crashing out

return Object.defineProperty(obj, name, {
configurable: false,
enumerable: true,
get() {
return isDarkMode() ? darkColors[name] : colors[name]
return `var(--color-${name})`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

use the css vars for inline styles also


// newarch @Override
override fun getTypedExportedConstants(): MutableMap<String, Any> {
@ReactMethod(isBlockingSynchronousMethod = true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

how the built in getConstants works is different between old and new arch and thats annoying for no reason. instead we just make a new function without any baggage

export const androidIsDeviceSecure: boolean = Kb.getConstants().androidIsDeviceSecure
export const androidIsTestDevice: boolean = Kb.getConstants().androidIsTestDevice
export const appVersionCode: string = Kb.getConstants().appVersionCode
export const appVersionName: string = Kb.getConstants().appVersionCode
Copy link
Contributor Author

Choose a reason for hiding this comment

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

typo!

}

const RNApp = React.memo(function RNApp() {
const s = Shared.useShared()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is where we do a lot of simplifying. on desktop+ios we use a native darkmode solution. on android we use a key to just rerender everything
previously we needed the nav rendered, then loaded, then tried to reload it intelligently
now we load and then render nav after all the initial state is known once

@chrisnojima chrisnojima merged commit 3493b60 into nojima/HOTPOT-76-again Jan 27, 2025
0 of 2 checks passed
@chrisnojima chrisnojima deleted the nojima/HOTPOT-76-fix-startup-nav-dark branch January 27, 2025 16:45
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.

1 participant