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

fix(InputMenu/SelectMenu): use by prop to compare objects & support dot notation in value-attribute #2566

Merged
merged 14 commits into from
Nov 10, 2024

Conversation

rdjanuar
Copy link
Collaborator

@rdjanuar rdjanuar commented Nov 8, 2024

πŸ”— Linked issue

Resolves #2028, resolves #2086, resolves #2342, resolves #2464

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@rdjanuar rdjanuar changed the title fix(SelectMenu): handle by prop fix(SelectMenu): use by prop to compare objects for selected values & add support for dot notation in value-attribute Nov 8, 2024
@rdjanuar rdjanuar marked this pull request as draft November 8, 2024 11:31
@rdjanuar rdjanuar marked this pull request as ready for review November 8, 2024 17:41
Copy link

cloudflare-workers-and-pages bot commented Nov 8, 2024

Deploying ui3 with Β Cloudflare Pages Β Cloudflare Pages

Latest commit: 633eac3
Status:🚫  Build failed.

View logs

@rdjanuar rdjanuar changed the title fix(SelectMenu): use by prop to compare objects for selected values & add support for dot notation in value-attribute fix(SelectMenu/InputMenu): use by prop to compare objects for selected values & add support for dot notation in value-attribute Nov 8, 2024
@benjamincanac benjamincanac changed the title fix(SelectMenu/InputMenu): use by prop to compare objects for selected values & add support for dot notation in value-attribute fix(InputMenu/SelectMenu): use by prop to compare objects & support dot notation in value-attribute Nov 10, 2024
@benjamincanac
Copy link
Member

@rdjanuar There seem to have an error with async search, try this: https://ui-dv8lpjs33-nuxt-js.vercel.app/components/select-menu#async-search

@benjamincanac
Copy link
Member

There is one more thing, when you make an async search the selected item disappears when it's no longer in the options.

You should change it so it behave like InputMenu, falling back to modelValue when there is no valueAttribute.

@benjamincanac
Copy link
Member

benjamincanac commented Nov 10, 2024

Do you think this #2342 is related? πŸ€”

@rdjanuar
Copy link
Collaborator Author

rdjanuar commented Nov 10, 2024

i already fix that issue u can check here https://ui-dc7qpq1zz-nuxt-js.vercel.app/components/select-menu#async-search.

I think the condition should rely on selected instead of modelValue @benjamincanac

Screenshot 2024-11-10 at 23 59 55

@rdjanuar
Copy link
Collaborator Author

rdjanuar commented Nov 10, 2024

I want to ask, is it normal for the USelectMenu component to fetch data when using props.search on mount? I think it should only interact/fetch when searching for a item @benjamincanac

Copy link
Member

There is a searchableLazy prop to handle this.

The count is not fixed:
https://volta.s3.fr-par.scw.cloud/Clean_Shot_2024_11_10_at_18_06_37_78ab4c6135.mp4

@rdjanuar
Copy link
Collaborator Author

rdjanuar commented Nov 10, 2024

There is a searchableLazy prop to handle this.

The count is not fixed: https://volta.s3.fr-par.scw.cloud/Clean_Shot_2024_11_10_at_18_06_37_78ab4c6135.mp4

You right @benjamincanac , we should rely on props.modelValue

@benjamincanac
Copy link
Member

@rdjanuar With your latest commits, your no longer handling the valueAttribute in the label πŸ€”

@rdjanuar
Copy link
Collaborator Author

Yes i remove it. Because i think condition props.optionAttribute relevant to handle label instead of props.valueAttribute @benjamincanac

Copy link
Member

Yeah but if a valueAttribute is set, the modelValue won't be an object it will be a string.

@benjamincanac
Copy link
Member

Well, I think we're good! Thanks a lot for your work on this 😊

@benjamincanac
Copy link
Member

I think there is one last issue when using async search + value-attribute but it's impossible to fix, I have the same on v3 πŸ˜…

@rdjanuar
Copy link
Collaborator Author

may i know the issue? @benjamincanac

@benjamincanac benjamincanac merged commit 7154254 into dev Nov 10, 2024
2 of 3 checks passed
@benjamincanac benjamincanac deleted the issue-2028 branch November 10, 2024 18:44
@benjamincanac
Copy link
Member

When you're doing an async search and set a value-attribute, the fallback to modelValue won't work since the items are gone when you search. Try this:

<script setup lang="ts">
const loading = ref(false)
const selected = ref([])

async function search(q: string) {
  loading.value = true

  const users: any[] = await $fetch('https://jsonplaceholder.typicode.com/users', { params: { q } })

  loading.value = false

  return users
}
</script>

<template>
  <USelectMenu
    v-model="selected"
    :loading="loading"
    :searchable="search"
    placeholder="Search for a user..."
    option-attribute="name"
    value-attribute="id"
  />
</template>

@rdjanuar
Copy link
Collaborator Author

When you're doing an async search and set a value-attribute, the fallback to modelValue won't work since the items are gone when you search. Try this:

<script setup lang="ts">
const loading = ref(false)
const selected = ref([])

async function search(q: string) {
  loading.value = true

  const users: any[] = await $fetch('https://jsonplaceholder.typicode.com/users', { params: { q } })

  loading.value = false

  return users
}
</script>

<template>
  <USelectMenu
    v-model="selected"
    :loading="loading"
    :searchable="search"
    placeholder="Search for a user..."
    option-attribute="name"
    value-attribute="id"
  />
</template>

i think the fallback work correctly, maybe u can try it after deployment dev is success

Copy link
Member

I just tried it locally, how can you fallback on a value that doesn't exist? The modelValue is equal to the id of the selected user.

@rdjanuar
Copy link
Collaborator Author

I just tried it locally, how can you fallback on a value that doesn't exist? The modelValue is equal to the id of the selected user.

your right, i already test it and the label disappear hahahaha. i have no idea to handle that :((

Copy link
Member

Don't worry about it, I really don't think its possible..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants