-
Notifications
You must be signed in to change notification settings - Fork 5
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
FR: Add way to easily access the selected item #5
Comments
I agree, there should be a first-class way to do this. I'll work on this when I get a chance. Feel free to start a PR if you have a solution in mind in the meantime! |
My current workaround has the significant downside of the selected item not being available on the first render, so in a SSR context the Select component wouldn't show the name of the selected option.
|
I've started working on this on the weekend, hope to be able to submit a PR later this week. My proposed solution would be quite similar to my user-land workaround. Unless I'm missing something, I don't think there's a way to make the selected item's data available in the parent on the first render. It's a bit of a shame, but I suppose it's a limitation of React's component model. And it doesn't have to be a deal-breaker, as component implementations that need to access the selected item on first render for SSR could always implement some sort of |
Thanks for the recent PR merge, @souporserious!
I was going to submit a PR for this and #7 yesterday, but didn't manage to wrap it up last night, and now there are merge conflicts as I also changed/cleaned up some of the demos a bit. 😅 But not a big deal. Here's the diff if you want to check it out: main...diondiondion:track-selected-item It took me several attempts at more complex approaches that wouldn't force a parent re-render before I ended up where I started: this simple implementation that's very close to the user-land solution. And it feels a bit like I'm coming to a similar conclusion to you in #7 – maybe this should be kept outside of the library's scope. 🤔 The other approaches I tried (creating a How attached are you to the hook-returning-a-hook API, btw? It seems like the library could be made much more "canonically Reacty" and more maintainable if the public API was made slightly less elegant: import {useItemlist, useItem} from 'use-item-list'
function Parent() {}
const itemList = useItemList({...})
return (
<Context.Provider value={itemList}>
...
</Context.Provider>
)
}
function Child() {}
const itemList = useContext(Context)
const item = useItem(itemList, {...})
return (
...
)
} The main reason why I feel this tradeoff is acceptable is that the majority of use-cases I have worked with so far required me to pass the |
Nice! Sorry about the merge conflicts, I was trying to think about how to tackle this last night. Do you think changing to your proposed API will help with getting selected items? It does seem about the same setup and probably not worth maintaining hooks that create hooks if we don't need to. |
One thought 🤔 probably against the grain of React, but maybe we can read in render and hydrate selected from there somehow for SSR. If I remember correctly, I think I tried this in the past with no luck on a stable solution. |
No worries, really. :) The conflicts were just in the demos/examples. I'll check out your changes soon, and if there's anything worth adding from my changes I'll make a separate PR for them. Best to keep those separate from features/fixes anyway.
Not really, no. The changed API would just (hopefully) make the package easier to break up into separate modules and reason about.
Hmm, making this work with SSR sounds really hard. Reading from React.Children.props works on the first render, but then we're back at a less flexible API that doesn't permit component composition or suspended items. What should definitely work is getting the selected item after first render, as soon as all the items are collected, so maybe even in the first I was also randomly reminded today that setting state during render can be fine in certain circumstances (link). Might be another option to look into. Apart from that we can always add an optional |
Hi, thanks for releasing the latest version of this lib!
I just started implementing a Select component using use-item-list and have run into an issue trying to display the label of the selected item on the button that opens the menu.
The issue is that the
Select
component should only receive thevalue
of the selected item, to mirror the native<select>
element's API. However, getting the selected item'stext
from itemList proved harder than I thought, asitemList.items
is initially empty, and there's no built-in way to access the currently selected item.For now I'm working around it with some extra state, passing the setter down into context and calling it from an effect within the
Option
component whenselected === true
.While this works fine for now, it seems like use-item-list should come with a built-in hook (e.g.
useSelectedItem
) that returns the selected item once it's "ready".The text was updated successfully, but these errors were encountered: