-
Notifications
You must be signed in to change notification settings - Fork 46
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 UrlSelect trigger value reset for non expo router projects #873
base: main
Are you sure you want to change the base?
Fix UrlSelect trigger value reset for non expo router projects #873
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
value: string; | ||
onValueChange: (newValue: string) => void; | ||
recentItems: UrlItem[]; | ||
items: UrlItem[]; | ||
disabled?: boolean; | ||
} | ||
|
||
function UrlSelect({ onValueChange, recentItems, items, value, disabled }: UrlSelectProps) { | ||
function UrlSelect({ onValueChange, recentItems, items, key, value, disabled }: UrlSelectProps) { |
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.
I believe that it only has been added in React 19 that you can receive key prop like this. I don't think in the current version in propagates down to the component
@@ -35,7 +36,7 @@ function UrlSelect({ onValueChange, recentItems, items, value, disabled }: UrlSe | |||
}; | |||
|
|||
return ( | |||
<Select.Root onValueChange={handleValueChange} value={value} disabled={disabled}> | |||
<Select.Root onValueChange={handleValueChange} key={key} value={value} disabled={disabled}> |
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.
It should be enough to do something along these lines, instead of having the whole logic for urlSelectKey in the parent component:
<Select.root key={value ? "has-value : "no-value"}>
@@ -117,8 +130,9 @@ function UrlBar({ disabled }: { disabled?: boolean }) { | |||
}} | |||
recentItems={recentUrlList} | |||
items={sortedUrlList} | |||
value={urlList[0]?.id} | |||
disabled={disabledAlsoWhenStarting || urlList.length < 2} | |||
key={urlSelectKey} |
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.
setting this here is enough to re-mount the whole component. It doesn't have to be accessed in the underlying component
This PR changes the UrlBar behaviour in projects not using Expo Router navigation.
When the user intends to close the preview by pressing the goHome button (or reload), the UrlSelect's trigger value needs to be reset to a placeholder ("/").
In this solution it is achieved by using the
urlSelectKey
andurlSelectValue
states to force a full re-render of the Radix Select when the key changes.Additionally, this update disables the
goBack
button for projects that do not use Expo Router.How Has This Been Tested:
Screen.Recording.2024-12-20.at.18.45.00.mov
Screen.Recording.2024-12-20.at.18.35.50.mov