-
-
Notifications
You must be signed in to change notification settings - Fork 174
feat: implement BFormRating component #2744
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
Conversation
Co-authored-by: Will Castillo [email protected]
- Introduce `clampedStars` computed to ensure at least 3 stars are always rendered - Replace all `props.stars` usages with `clampedStars.value` in template and computed icon logic - Add `showValue` and `ShowValueMax` props, plus `displayValueText` computed, to render “current” or “current/max” next to stars - Wire `showValue`/`ShowValueMax` into a single `<span>` for value output
- Add `BFormRating: '/components/BFormRating'` to `componentsWithExternalPath` in BootstrapVueOptions.ts
- Add variant props for customization - Modify iconColor for only 1 color support instead of 2
- Support `size="sm" | "lg"` keywords with `computedSize`, plus custom CSS units - Update `displayValueText` to handle `showValue`, `showValueMax`, and `precision` (0–n decimals) - Compute `roundedValue` based on `precision` prop - Compute `iconColors` array to apply Bootstrap `variant` classes or inline custom `color` - Update template to bind `computedSize`, `iconColors`, and `displayValueText`
…states, sizing, colors, and value display
Create unit test file for new BFormRating component
- Add `FormRating.data.ts` with prop metadata for BFormRating - Create `form-rating.md` page with overview, usage examples and API table - Update `TableOfContentsNav.vue` to include the new Form Rating section - Add demo components for all features: - `RatingInteractive.vue` (interactive mode) - `RatingNonInteractive.vue` (readonly mode) - `RatingPrecision.vue` (precision) - `RatingStars.vue` (custom star count) - `RatingSizing.vue` (size presets) - `RatingValue.vue` (show-value) - `RatingValueMax.vue` (show-value-max) - `RatingVariant.vue` (Bootstrap variants) - `RatingCustomColor.vue` (custom color)
docs(form-rating): add documentation, data file & live demos
- Add margin for the star icon. - Change sizing for 'lg', 'md' to fit Bootstrap convention.
…proved accessibility. Removed hover state logic and added tabindex for better interaction. Updated styles for star icons on hover.
…for testing - Implemented clear button to reset rating - Added corresponding styles in _form-rating.scss - Developed unit tests for new functionalities - Expose hoverValue for testing
Clear button and Merge branch 'bootstrap-vue-next:main' into clear-button
Added noBorder prop to BFormRating with SCSS border styling and unit tests
dwgray
left a comment
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.
Please address the AI comments. I did a quick pass and they all seem valid (but think them through, I only did a quick pass).
Also, please update the BFormRatings section of migration-guide.md with (a) information about what client code would need to change to consume this component - the most obvious one is customer icons/slots, but there may be others and (b) not yet implemented notes for the featurelets that you didn't get to - once things have solidified a bit more, you can open a bug against the NYI features.
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
♻️ Duplicate comments (1)
apps/docs/src/docs/components/form-rating.md (1)
82-82: Add missing inline mode demo
The documentation describes the inline mode but doesn’t include the corresponding demo embed. Please insert the demo reference here to match other feature sections.Proposed diff:
@@ -82,1 +82,2 - Simply set the `inline` prop to `true` to render the component in inline mode: + Simply set the `inline` prop to `true` to render the component in inline mode: + <<< DEMO ./demo/RatingInline.vue
🧹 Nitpick comments (1)
apps/docs/src/docs/components/form-rating.md (1)
54-54: Improve comma usage after introductory phrases
Add missing commas for consistency and smoother readability.- By default `<BFormRating>` does not display the current numerical value. + By default, `<BFormRating>` does not display the current numerical value. - Optionally show the maximum rating possible by also setting the prop `show-value-max` to `true`: + Optionally, show the maximum rating possible by also setting the prop `show-value-max` to `true`: - In some situations you may prefer the custom input to occupy only the space required for its contents. + In some situations, you may prefer the custom input to occupy only the space required for its contents.Also applies to: 67-67, 81-81
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/docs/src/data/components/FormRating.data.ts(1 hunks)apps/docs/src/docs/components/demo/RatingCustomColor.vue(1 hunks)apps/docs/src/docs/components/demo/RatingSizing.vue(1 hunks)apps/docs/src/docs/components/form-rating.md(1 hunks)packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.spec.ts(1 hunks)packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.vue(1 hunks)packages/bootstrap-vue-next/src/components/BFormRating/_form-rating.scss(1 hunks)packages/bootstrap-vue-next/src/styles/styles.scss(1 hunks)packages/bootstrap-vue-next/src/types/ComponentProps.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/bootstrap-vue-next/src/styles/styles.scss
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/docs/src/docs/components/demo/RatingSizing.vue
- packages/bootstrap-vue-next/src/components/BFormRating/_form-rating.scss
- apps/docs/src/docs/components/demo/RatingCustomColor.vue
- packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.spec.ts
- apps/docs/src/data/components/FormRating.data.ts
- packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.vue
- packages/bootstrap-vue-next/src/types/ComponentProps.ts
🧰 Additional context used
🪛 LanguageTool
apps/docs/src/docs/components/form-rating.md
[uncategorized] ~53-~53: Did you mean: “By default,”?
Context: ..../demo/RatingStars.vue ### Show value By default <BFormRating> does not display the cu...
(BY_DEFAULT_COMMA)
[uncategorized] ~66-~66: Possible missing comma found.
Context: ...Precision.vue #### Show maximum value Optionally show the maximum rating possible by als...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~80-~80: It appears that a comma is missing.
Context: ... width of the parent container. In some situations you may prefer the custom input to occu...
(DURING_THAT_TIME_COMMA)
…yling - Removed mouse move event handling from BFormRating component. - Updated scaling effect for clear icon and added hover effect for star icons in SCSS. - Enhanced BFormRatingProps to support specific size and variant options.
- Changed background color from white to var(--bs-body-bg) for improved theming consistency.
- Updated default values and types for props in BFormRating, including `modelValue`, `precision`, `readonly`, `stars`, `variant`, `size`, and new props `noBorder`, `showClear`, and `inline`. - Added detailed migration guide and component documentation reflecting the new features and changes. - Introduced demo components showcasing the new `no-border` and `show-clear` functionalities. - Updated existing demo components to reflect changes in props and usage.
- Introduced `inline` prop to BFormRating for flexible layout options. - Updated SCSS styles for better spacing and hover effects on clear icons and stars. - Adjusted default sizes for star icons and refined margin settings. - Add unit tests to cover new keyboard navigation, precision features, and inline. - Updated component props to reflect changes in size and inline behavior.
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: 2
🧹 Nitpick comments (1)
apps/docs/src/docs/components/demo/RatingCustomIcon.vue (1)
7-37: Hide decorative SVGs from assistive tech.
<svg>icons are purely visual; withoutaria-hidden="true"they are exposed as unnamed graphics, cluttering the accessibility tree.- <svg + <svg + aria-hidden="true" class="custom-icon" @@ - <svg + <svg + aria-hidden="true" class="custom-icon"Add the attribute to both filled and empty hearts (or
role="presentation").
No functional change, only better a11y semantics.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/docs/src/data/components/FormRating.data.ts(1 hunks)apps/docs/src/docs/components/demo/RatingBorder.vue(1 hunks)apps/docs/src/docs/components/demo/RatingClear.vue(1 hunks)apps/docs/src/docs/components/demo/RatingCustomIcon.vue(1 hunks)apps/docs/src/docs/components/demo/RatingInline.vue(1 hunks)apps/docs/src/docs/components/demo/RatingVariant.vue(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/docs/src/docs/components/demo/RatingClear.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/docs/src/data/components/FormRating.data.ts
🔇 Additional comments (1)
apps/docs/src/docs/components/demo/RatingBorder.vue (1)
2-11: Looks good – clear, labelled, and unique IDs.All three borderless examples have proper labels and unique
ids; the demo cleanly showcases the prop. 👍
…lowing CodeRabbit recommendation - Added `aria-hidden="true"` to SVG icons in RatingCustomIcon for better accessibility. - Assigned `id` to BFormRating in RatingInline for improved label association. - Included `aria-label` attributes for each BFormRating variant in RatingVariant to enhance screen reader support.
dwgray
left a comment
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.
Looks great. Just fix the couple of small things in the migration guide and make sure the CR comments are cleaned up and we should be ready to go.
packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.vue
Outdated
Show resolved
Hide resolved
- Documented the availability of `BFormRating` in BootstrapVueNext, highlighting adherence to Vue 3's v-model conventions. - Listed deprecated icon-related props and introduced new customization approaches for icons. - Clarified features not yet implemented in `BFormRating` and provided links to relevant migration resources.
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
♻️ Duplicate comments (1)
apps/docs/src/docs/migration-guide.md (1)
416-424: Deprecated icon props listed explicitly
This change addresses the previous feedback by namingicon-empty,icon-half,icon-full, andicon-clear.🧰 Tools
🪛 LanguageTool
[uncategorized] ~420-~420: Loose punctuation mark.
Context: ...ue have been deprecated: -icon-empty: For specifying empty star icon - `icon-...(UNLIKELY_OPENING_PUNCTUATION)
🧹 Nitpick comments (3)
apps/docs/src/docs/migration-guide.md (3)
409-415: Enhance unimplemented-features list
Consider linking each unimplemented feature to its corresponding issue or roadmap entry so users can track progress (e.g. issue #2051 fordisabled, #… for others).
425-430: Link to custom-icon slot examples
You may want to add a direct link or FRAGMENT reference to the demo page showing scoped‐slot icon overrides (e.g.demo/FormRatingCustomIcon.vue) for quicker discovery.
657-657: Fix sentence capitalization
Change “been deprecated along with thedisabledandenabledevents.” to “Has been deprecated along with thedisabledandenabledevents.” for grammatical consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/docs/src/docs/migration-guide.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/docs/src/docs/migration-guide.md
[uncategorized] ~420-~420: Loose punctuation mark.
Context: ...ue have been deprecated: - icon-empty: For specifying empty star icon - `icon-...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (2)
apps/docs/src/docs/migration-guide.md (2)
406-407: Document BFormRating section clearly
The new BFormRating section correctly introduces availability and links to the Vue 3v-modelguide.
651-651: Approve popover variant deprecation note
The deprecation of thevariantprop is clearly documented with a link to the new styling approach.
commit: |
dwgray
left a comment
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.
Looks good to me!
dwgray
left a comment
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.
I'm going to confirm that we've got the correct contributors listed before letting this through.
VividLemon
left a comment
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.
I apologize for doing this last minute. All seems simple enough fixes (and could be fixed later by someone if they wish)
packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.vue
Outdated
Show resolved
Hide resolved
packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.vue
Outdated
Show resolved
Hide resolved
packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.vue
Outdated
Show resolved
Hide resolved
packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.vue
Outdated
Show resolved
Hide resolved
packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.vue
Outdated
Show resolved
Hide resolved
packages/bootstrap-vue-next/src/components/BFormRating/BFormRating.vue
Outdated
Show resolved
Hide resolved
- Updated the import path for BFormRating in RatingCustomIcon.vue to improve clarity. - Refactored class binding in BFormRating.vue to use computed properties for better maintainability. - Adjusted props handling in BFormRating to exclude undefined modelValue and streamline defaults. - Removed unnecessary styles and comments to clean up the component code.
I've looked through these changes, and they've all been addressed based on other conversations with VividLemon, I'm going to push this PR through and will clean up later if I missed anything.
* 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)
Describe the PR
Implemented the BFormRating component. Allows users to select a star rating, supports a customizable number of stars, includes a read-only mode, color variation, sizes, and displays the value selected and max value. Added unit tests to verify functionality and included clear documentation. Fixes #2051
Contributor: Huy Nguyen - https://github.com/hnben
Contributor: William Castillo - https://github.com/william-castillo-jr
Contributor: Huy Le - https://github.com/jackle500
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
Documentation
Tests