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

feat: list.insert #270

Merged
merged 6 commits into from
Aug 28, 2023
Merged

feat: list.insert #270

merged 6 commits into from
Aug 28, 2023

Conversation

albert-schilling
Copy link
Contributor

This PR extends the list helper with an insert (in between) method.

{/* To insert a new row with optional defaultValue at a given index */}
<button {...list.insert('name', { defaultValue, index })}>Insert</button>

@albert-schilling albert-schilling changed the title List insert feat: list.insert Aug 21, 2023
@edmundhung
Copy link
Owner

Thanks @albert-schilling! I am happy with the new changes. But Can you undo the version bump? I will prefer taking care of it separately.

Now that we have insert, I might want to deprecate append / prepend so there won't be several ways to do the same thing. Perhaps making index optional and default to append. Wdyt?

@albert-schilling
Copy link
Contributor Author

Thanks @albert-schilling! I am happy with the new changes. But Can you undo the version bump? I will prefer taking care of it separately.

Now that we have insert, I might want to deprecate append / prepend so there won't be several ways to do the same thing. Perhaps making index optional and default to append. Wdyt?

Thanks for the quick review.
I agree. Keeping the interface as small as sensible is always good.

I updated the guide and added the deprecation hint, but I did not find a way to add the deprecation warning in the type annotation of the append and prepend methods. Do you know how to do that?

I also changed all tests to test insert instead of prepend and append. Is that okay with you?

@edmundhung edmundhung changed the base branch from main to next August 28, 2023 18:21
@edmundhung edmundhung merged commit 5462fd0 into edmundhung:next Aug 28, 2023
1 check passed
@edmundhung
Copy link
Owner

Thank you! @albert-schilling

@albert-schilling
Copy link
Contributor Author

Thank you! @albert-schilling

Thank you for reviewing and merging it. Looking forward to using it in our code base

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.

3 participants