-
-
Notifications
You must be signed in to change notification settings - Fork 174
docs(BFormRating): Parity pass #2749
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
docs(BFormRating): Parity pass #2749
Conversation
|
|
WalkthroughThe changes reorganize and extend the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BFormRating
participant useId
User->>BFormRating: Mounts component with props (id, inline, etc.)
BFormRating->>useId: Compute id (props.id or fallback)
useId-->>BFormRating: Returns computedId
BFormRating-->>User: Renders with computedId and updated props
Possibly related issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.vue (1)
210-213: Clear button sets the value to0, docs saynull
The docs (see form-rating.md) state that clicking the clear icon will setmodelValuetonull, but the implementation forces it to0.This is a behavioural contract break:
BFormRatingProps['modelValue']is declared asnumber, so assigningnullis currently impossible.- Users relying on the documentation will mis-handle the cleared state.
Pick one of the two approaches and make it consistent across code, types and docs.
A minimal code-side fix keeping the current type would be:-function clearRating() { - hoverValue.value = null - localValue.value = 0 -} +function clearRating() { + hoverValue.value = null + localValue.value = null as unknown as number // or widen the prop/type to number | null +}…but widening
modelValuetonumber | nullis the cleaner solution.
Whichever route you choose, update the docs and prop typing together.
🧹 Nitpick comments (3)
packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.vue (1)
99-100: Prefix'input'may collide with other inputs
useId(() => props.id, 'input')re-uses the very common prefix input. If a page renders many components that rely on the same fallback prefix, the generated IDs become long (“input-1”, “input-2”…), but more importantly they lose semantic meaning when debugging.
Consider a component-specific prefix such as'rating'to keep the DOM easier to scan.-const computedId = useId(() => props.id, 'input') +const computedId = useId(() => props.id, 'rating')apps/docs/src/data/components/FormRating.data.ts (2)
94-111: Slot description still references removed props
iconFull,iconHalf,iconEmptywere removed from both the component and the public props, yet they’re still listed in the slot description. Drop them to avoid confusion and stay in sync with the new API.
63-64:defaultvalue should be numeric, not string
Thesizeprop’s default is correctly a string ('1rem'), but for thestarsprop the default is defined as the string'5'.
Use a number to match the runtime default and avoid type-drift in generated docs.- default: '5', + default: 5,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/docs/src/data/components/FormRating.data.ts(4 hunks)apps/docs/src/docs/components/form-rating.md(1 hunks)packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.vue(2 hunks)packages/bootstrap-vue-next/src/types/ComponentProps.ts(1 hunks)
🔇 Additional comments (2)
packages/bootstrap-vue-next/src/types/ComponentProps.ts (1)
358-360: Good addition ofidandinlineprops
The interface now mirrors the new component behaviour, keeping type-safety intact. 👍apps/docs/src/docs/components/form-rating.md (1)
172-176: Documentation does not match implementation
The note claims the root element is an<output>but the Vue file renders a<div>.
Please update the docs or switch the root element so that users do not copy incorrect markup.
* upstream/main: docs: clean up css selector docs: fix on-this-page when examples use header tags docs(migration): add component aliases guidelines (bootstrap-vue-next#2771) chore: upgrade dependencies and address all lint warnings (bootstrap-vue-next#2785) chore: release main (bootstrap-vue-next#2769) fix(BDropdown): prevent hydration warning in nuxt production mode (bootstrap-vue-next#2768) docs(BTabs): Updates based on v-model changes (bootstrap-vue-next#2760) docs(table): fix missing anchor in `BTableLite` and `BTableSimple` links (bootstrap-vue-next#2759) docs(BFormRating): Parity pass (bootstrap-vue-next#2749) docs: fix typo in breadcrumb documentation (bootstrap-vue-next#2756) docs: Fix empty-text and empty-filtered-text description as they require show-empty to be set (bootstrap-vue-next#2755) fix InputGroupXPend.vue chore: release main (bootstrap-vue-next#2748) feat: implement BFormRating component (bootstrap-vue-next#2744) chore: release main fix(BLink): move active class to BNavItem (bootstrap-vue-next#2747) feat(BDropdown): allow setting icon prop on nested BButton (bootstrap-vue-next#2746) fix(BToast): close BToast correctly if modelValue is changed from number to false (bootstrap-vue-next#2745)
…keover * upstream/main: (21 commits) feat(b-form-rating): add `icon-clear` slot (#2809) docs: fix dead links, enable rule (#2808) chore: release main (#2801) docs: Fix navbar collapse behavior (#2802) docs: fix code tabs on getting started and icons pages (#2805) docs: fix missing data and use a stronger schema to catch missing errors (#2806) feat(BPopover): add explicit props for focus and hover triggers (#2795) fix(BNavbar): changed autoClose to noAutoClose and fix the documention mistake about it. fix: ssr in scrollspy docs: clean up css selector docs: fix on-this-page when examples use header tags docs(migration): add component aliases guidelines (#2771) chore: upgrade dependencies and address all lint warnings (#2785) chore: release main (#2769) fix(BDropdown): prevent hydration warning in nuxt production mode (#2768) docs(BTabs): Updates based on v-model changes (#2760) docs(table): fix missing anchor in `BTableLite` and `BTableSimple` links (#2759) docs(BFormRating): Parity pass (#2749) docs: fix typo in breadcrumb documentation (#2756) docs: Fix empty-text and empty-filtered-text description as they require show-empty to be set (#2755) ...
Describe the PR
Parity pass for BFormRating
idpropicon*propsSmall replication
See the docs
PR checklist
What kind of change does this PR introduce? (check at least one)
fix(...)feat(...)fix(...)docs(...)The PR fulfills these requirements:
CHANGELOGis generated from these messages, and determines the next version type. Pull requests that do not follow conventional commits or do not have an override will be deniedSummary by CodeRabbit
New Features
Improvements
Removals