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(useCycleList): allow receiving reactive list #2864

Merged
merged 2 commits into from Mar 14, 2023

Conversation

hjJunior
Copy link
Contributor

Description

Allow useCycleList to receive a "ref" list value, this will allow the cycle list to be reactive

Additional context

This also adds unit tests for useCycleList


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@hjJunior hjJunior force-pushed the hjjunior/allow-refs-on-use-cycle-list branch from 01d141b to f7e58ef Compare March 13, 2023 18:45
@hjJunior hjJunior changed the title Allow useCycleList to receive reactive list and add tests feat(useCycleList): Allow receiving reactive list Mar 13, 2023
@antfu antfu changed the title feat(useCycleList): Allow receiving reactive list feat(useCycleList): allow receiving reactive list Mar 14, 2023
@antfu antfu enabled auto-merge (squash) March 14, 2023 16:36
@antfu antfu merged commit b65c2cc into vueuse:main Mar 14, 2023
@hjJunior hjJunior deleted the hjjunior/allow-refs-on-use-cycle-list branch March 14, 2023 17:05
@chrisspiegl
Copy link

I just tested v10 beta and noticed that my previous normal array [] of list items did no longer work with the new update.

I then made my list a ref([]) and it did work again.

So, to me, this looks like a breaking change. And as it stands it only supports ref([]) and no longer supports standard arrays?

@antfu
Copy link
Member

antfu commented Mar 16, 2023

@chrisspiegl what do you mean "no longer work"? Can you add your usage to https://github.com/vueuse/vueuse/blob/b65c2ccdb7db7e3e54d9e7145aec4ed74fe4f855/packages/core/useCycleList/index.test.ts so we could see how we can fix it?

@chrisspiegl
Copy link

chrisspiegl commented Mar 17, 2023

@antfu & @hjJunior, thank you for your reply and I will try my best to help describe/show the issue I ran into.

I updated the Cloned and adjusted the PlayCode example.

I learned two things while doing so:

  • I am using useCycleList with an array of objects (sortOrders).
  • Using an array of strings seems to work with both the @latest and @beta.
  • With a `ref(sortOrders) with sortOrders being an array of objects it also works in both version.

Expectation:

An array of objets should also work when it is not a ref()? Just like the array of strings does.

Here is my example which does not work: https://playcode.io/1314120

Result:

It's not really a change which is different between v9 and v10… but it's something useCycleList wasn't able to do to begin with 🙈? But should be able to do (cycle through an array of objects).

@Alfred-Skyblue Alfred-Skyblue mentioned this pull request Apr 7, 2023
5 tasks
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.

None yet

3 participants