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(CheckboxGroup): new component #895

Closed
wants to merge 3 commits into from
Closed

feat(CheckboxGroup): new component #895

wants to merge 3 commits into from

Conversation

sandros94
Copy link
Collaborator

πŸ”— Linked issue

I decided not to create an issue for such little task

❓ 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

Much like the new RadioGroup component it could be useful (both as a standard as well as a dev experience) to have a similar component for Checkboxes.

πŸ“ Checklist

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

@vercel
Copy link

vercel bot commented Oct 31, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated (UTC)
ui ❌ Failed (Inspect) Oct 31, 2023 2:57pm

@sandros94
Copy link
Collaborator Author

I'm not fully sure how to update the docs, so I'm happy to get directions. But I marked this as ready to merge anyway, since it could need something else done that I'm not aware of.

Also, I've noticed that the Radio page disappeared once RadioGroup got released.

@benjamincanac
Copy link
Member

Good idea! I guess we could do like on #881 and make the label size configurable, by taking into account this fix #894.

@sandros94
Copy link
Collaborator Author

Good idea! I guess we could do like on #881 and make the label size configurable, by taking into account this fix #894.

True, I did miss to change checkbox in ui.config.ts to make this work

@ineshbose
Copy link
Member

I'm unable to push on this PR to fix the conflicts. πŸ€”

! [remote rejected] HEAD -> dev (refusing to allow a GitHub App to create or update workflow .github/workflows/ci-dev.yml without workflows permission)

@sandros94
Copy link
Collaborator Author

sandros94 commented Dec 1, 2023

I'm unable to push on this PR to fix the conflicts. πŸ€”

! [remote rejected] HEAD -> dev (refusing to allow a GitHub App to create or update workflow .github/workflows/ci-dev.yml without workflows permission)

I do remember noticing a failed run (due to not having permissions) as soon as I forked the repo

UPDATE: but now if I want to check what could be causing it I do correctly see that workflows are disabled on my side

@benjamincanac
Copy link
Member

Sorry but I won't be merging this, I've thought about it and it doesn't really make that much sense to have a CheckboxGroup component whereas the RadioGroup has some logic tied to it. This one would be just a shortcut for a v-for and therefore a bit overkill in my opinion.

@sandros94
Copy link
Collaborator Author

Sorry but I won't be merging this, I've thought about it and it doesn't really make that much sense to have a CheckboxGroup component whereas the RadioGroup has some logic tied to it. This one would be just a shortcut for a v-for and therefore a bit overkill in my opinion.

No problem. As I said in the PR description this was only for streamlining naming convention and developer experience at a conceptual level, thus not required.

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