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(CommandPalette): keep ignoreFilter groups at their place #2833

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

benjamincanac
Copy link
Member

Related to #2832

πŸ”— Linked issue

❓ 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.

Copy link

Deploying ui3 with Β Cloudflare Pages Β Cloudflare Pages

Latest commit: 5586b36
Status:Β βœ…Β  Deploy successful!
Preview URL: https://ba333e49.ui-6q2.pages.dev
Branch Preview URL: https://fix-command-palette-order.ui-6q2.pages.dev

View logs

Copy link

pkg-pr-new bot commented Dec 4, 2024

npm i https://pkg.pr.new/@nuxt/ui@2833

commit: 5586b36

@benjamincanac
Copy link
Member Author

@maximepvrt I remember we talked about this a few months back, do you have an opinion on this?

@maximepvrt
Copy link
Contributor

@benjamincanac Yes, absolutely! It's great to see this being implemented : my original MR (#1457) 😊 Also, your memory is incredible!

@benjamincanac
Copy link
Member Author

Haha thanks, so this makes sense to you to keep original order of non fuse groups only?

@maximepvrt
Copy link
Contributor

Yes, keeping the original order of non-fuse groups makes more sense to me as well it feels more logical. In my original MR, I proposed this approach mainly to avoid a breaking change in v2.

If needed, we could also consider adding an option to define a placement strategy for the groups. What do you think? 😊

@benjamincanac
Copy link
Member Author

If we do this, let's add a position field on the group instead of what I did so it can be done with both fuse and non-fuse groups, wdyt?

@maximepvrt
Copy link
Contributor

@benjamincanac Are you suggesting the order would follow the fuse logic unless manual indexes are provided?

What happens to groups without indexes in cases where some do have indexes? Would they appear first, last, or follow another logic?

Copy link
Member Author

Yes, they'd keep their position based on the Fuse results I guess, not sure πŸ˜…

@benjamincanac benjamincanac marked this pull request as ready for review December 9, 2024 09:17
@benjamincanac benjamincanac merged commit 3b9ca22 into v3 Dec 9, 2024
3 checks passed
@benjamincanac benjamincanac deleted the fix/command-palette-order branch December 9, 2024 09:17
@benjamincanac benjamincanac added the v3 #1289 label Dec 9, 2024 — with Volta.net
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3 #1289
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants