10000 fix: make `select.web` consistent by bryanmylee · Pull Request #57 · roninoss/rn-primitives · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: make select.web consistent #57

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bryanmylee
Copy link
Contributor
@bryanmylee bryanmylee commented Feb 1, 2025

This solves #26 in making the <Select> component consistent on web and native.

For each <Item> component, keep track of its value to label mapping on the <Root> component by passing a MutableRefObject that stores the mapping from <Root> to <Item> via context.

On render, <Item> sets the label for its value. <Root> can then call onValueChange with labelForValueRef.current[val] ?? val, allowing feature parity with the native implementation.

This approach works perfectly. Since Item must be rendered before it can be interacted with and emit events, this always ensures that labelForValueRef[val] exists before onStrValueChange is called. Furthermore, since the value and defaultValue prop requires { value: string, label: string }, the initial render also does not have any issues.

Copy link
vercel bot commented Feb 1, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
rn-primitives ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 4, 2025 3:02pm

@bryanmylee
Copy link
Contributor Author

Added another fix to solve #58.

Copy link
Collaborator
@mrzachnugent mrzachnugent left a comment

Choose a reason for hiding this comment

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

@bryanmylee Thanks for another contribution.

For each `<Item>` component, keep track of its `value` to `label`
mapping on the `<Root>` component by passing a `MutableRefObject`
that stores the mapping from `<Root>` to `<Item>` via context.

On render, `<Item>` sets the label for its value. `<Root>` can then call
`onValueChange` with `labelForValueRef.current[val] ?? val`, allowing
feature parity with the native implementation.

This approach works perfectly. Since `Item` must be rendered before it
can be interacted with and emit events, this always ensures that
`labelForValueRef[val]` exists before `onStrValueChange` is called.
Furthermore, since the `value` and `defaultValue` prop requires `{
value: string, label: string }`, the initial render also does not have
any issues.
@bryanmylee
Copy link
Contributor Author

closeOnPress is not handled on the web version of <Select>

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.

2 participants
0