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

WIP Fix: list section children are not dynamic #62

Conversation

thetarnav
Copy link
Member

Fixes #59

TODO:

  • Pass select tests
  • Add tests to collection package

@changeset-bot
Copy link

changeset-bot bot commented Jul 20, 2022

⚠️ No Changeset found

Latest commit: a804a4c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@thetarnav thetarnav linked an issue Jul 20, 2022 that may be closed by this pull request
@thetarnav thetarnav changed the base branch from fix/list-section-children-are-not-dynamic to main July 20, 2022 18:48
@thetarnav thetarnav marked this pull request as ready for review July 21, 2022 18:03
@thetarnav
Copy link
Member Author

thetarnav commented Jul 21, 2022

Having the children dynamic required only small changes in two places:

  1. createCollection
  2. createListState

Accessing children just needed to be read under a tracking scope.
The problem was that calling resolvedChildren wasn't tracking nested changes. e.g. if you would add another section it would probably track, but not if you add a new item under that section.
Maybe a different primitive would be useful here? Like custom children() helper that will listen to nested changes too.

Here I just added path aliasing for vite, so that you don't have to rebuild after changing stuff in other deps.

As for the select and the failing tests, the createSelect isn't handling that initial focus after opening the options list properly it seems.
Do you want to merge it to some wip branch and take it from here? I don't really know what to do more.

@fabien-ml
Copy link
Member

You can merge into develop, will take a look at it when I have time.

@thetarnav thetarnav changed the base branch from main to develop July 21, 2022 21:08
@thetarnav thetarnav merged commit 860cfec into solidjs-community:develop Jul 21, 2022
@thetarnav thetarnav deleted the fix/list-section-children-are-not-dynamic branch July 21, 2022 21:17
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.

Bug: List section children are not dynamic
2 participants