Support fragments in collections #6430
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Thanks for the PR, we'll need tests for this.
Something to look out for is caching of entire children of the fragment when they would each be individually cached if not in a fragment.
Another is making sure that mixed structures like these work
<Parent>
<Item></Item>
<>
<Item></Item>
</>
</Parent>
<Parent>
<>
<Item></Item>
<Item></Item>
</>
</Parent>
solimant
force-pushed
the
support_fragments
branch
2 times, most recently
from
7731fa0
to
1bfbc8b
Compare
solimant
force-pushed
the
support_fragments
branch
from
1bfbc8b
to
2a61762
Compare
|
@snowystinger - Rob, thank you for the review, and sorry for the delay in addressing the feedback. I've included tests, and revised the implementation a little which I think should address the use of the cache. Happy to address any further feedback. |
snowystinger
approved these changes
Jul 16, 2024
|
No worries, this looks reasonable. We'll get someone else to review it after our next release. Hopefully next week. Thanks @solimant ! |
devongovett
force-pushed
the
main
branch
2 times, most recently
from
8cb1a5d
to
3013156
Compare
reidbarber
added a commit
that referenced
this pull request
Aug 21, 2024
Co-authored-by: Reid Barber <reid@reidbarber.com> Co-authored-by: Robert Snow <rsnow@adobe.com>
5 tasks
reidbarber
added a commit
that referenced
this pull request
Aug 21, 2024
* Update Picker placeholder to be shorter (#6796) * feat: Support fragments in collections (#6430) Co-authored-by: Reid Barber <reid@reidbarber.com> Co-authored-by: Robert Snow <rsnow@adobe.com> * Exposing prop disabledBehavior to TableView (#6832) * fix/bug useTablist #5996 (#6023) * fix/bug useTabist #5996 and added tests * Extract `ToggleStateProps` type to use only what is needed in `useToggleState` (#3836) * Extract `ToggleStateProps` type to use only what is needed in `useToggleState` --------- Co-authored-by: Robert Snow <rsnow@adobe.com> Co-authored-by: solimant <solimant@users.noreply.github.com> Co-authored-by: Kyle Taborski <ktabors@yahoo.com> Co-authored-by: Medhashis Maiti <129734281+mdhmaiti@users.noreply.github.com> Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: