Add missing overflow behaviors to BitPivot (#432)#12535
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds Menu and Slide overflow behaviors to ChangesBitPivot Overflow Behavior
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ResizeObserver
participant PivotInstance
participant BitPivot as BitPivot (.NET)
ResizeObserver->>PivotInstance: resize triggers update()
PivotInstance->>PivotInstance: updateMenu() / updateSlide()
PivotInstance->>BitPivot: OnSetOverflowItems(indexes)
PivotInstance->>BitPivot: OnSetSlideState(hasOverflow, atStart, atEnd)
BitPivot->>BitPivot: StateHasChanged()
BitPivot->>PivotInstance: slide(forward)
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Pivot/BitPivotTests.cs (1)
61-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the overflow button label.
This still only asserts a regular pivot item’s
aria-label. The new Menu branch adds a separate icon-only overflow trigger, so a regression inOverflowAriaLabelwiring would pass this suite.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Pivot/BitPivotTests.cs` around lines 61 - 72, Add a test for the overflow button’s aria-label, since BitPivotAriaLabelTest only covers regular pivot items. Update BitPivotTests to render a BitPivot with overflow enabled and assert the overflow trigger button uses the OverflowAriaLabel value, so regressions in BitPivot/OverflowAriaLabel wiring are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/Pivot/BitPivot.razor`:
- Around line 33-40: The overflow trigger in BitPivot is exposing
OverflowAriaLabel directly on an icon-only button, so it needs a built-in
accessible fallback instead of defaulting to null. Update the BitPivot
component’s overflow button markup and the OverflowAriaLabel parameter handling
so the control uses a sensible default label when no override is provided, while
still allowing consumers to supply their own value.
- Around line 18-24: The icon-only slide buttons in BitPivot.razor are unnamed
controls, so add accessible labels to both the previous and next buttons. Update
the button markup around the Slide(false) and Slide(true) handlers to include
localized aria-label text, and add matching public parameters or resource-backed
values if the component doesn’t already expose them. Keep the labels symmetric
and ensure the existing prevIcon/nextIcon usage remains unchanged.
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/Pivot/BitPivot.razor.cs`:
- Around line 87-124: Add accessible names for the icon-only overflow buttons in
BitPivot so slide navigation and menu controls are labeled by default. Update
BitPivot.razor and the BitPivot component contract to introduce/fallback
aria-label parameters for the previous and next slide buttons, and make
OverflowAriaLabel non-null-safe by providing a default label when none is
supplied. Wire these labels into the existing icon-only button markup so the new
overflow modes are accessible without requiring consumers to set every label
manually.
- Around line 241-293: The Pivot JS instance is only reinitialized when
OverflowBehavior changes, but OnAfterRenderAsync also passes the current RTL
state into BitPivotSetup, so switching Dir at runtime leaves the existing
PivotInstance using stale direction logic. Update the teardown/recreate
condition in OnAfterRenderAsync to also detect changes in the effective RTL
state (the Dir is BitDir.Rtl value), and when that changes dispose the current
JS setup and rebuild it just like a behavior change. Keep the refresh path only
for cases where both behavior and RTL state are unchanged.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/Pivot/BitPivotDemo.razor.cs`:
- Around line 1560-1589: The `example15RazorCode` sample for `BitPivot` does not
reproduce overflow because it omits the narrow-width wrapper used by the live
demo. Update the snippet to include the same constrained container styling
around each `BitPivot` example so `OverflowBehavior.Menu`, `Slide`, and `Scroll`
actually trigger in a typical copy/paste scenario and match the demo behavior.
---
Nitpick comments:
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Pivot/BitPivotTests.cs`:
- Around line 61-72: Add a test for the overflow button’s aria-label, since
BitPivotAriaLabelTest only covers regular pivot items. Update BitPivotTests to
render a BitPivot with overflow enabled and assert the overflow trigger button
uses the OverflowAriaLabel value, so regressions in BitPivot/OverflowAriaLabel
wiring are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c5341a70-bc3a-469f-be1e-9b6c1a17cfca
📒 Files selected for processing (10)
src/BlazorUI/Bit.BlazorUI/Components/Navs/Pivot/BitPivot.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Pivot/BitPivot.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Pivot/BitPivot.scsssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Pivot/BitPivotClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Pivot/BitPivotOverflowBehavior.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/PivotJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Pivot.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/Pivot/BitPivotDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/Pivot/BitPivotDemo.razor.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Pivot/BitPivotTests.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/Pivot/BitPivotDemo.razor.cs`:
- Around line 82-85: The Pivot demo property descriptions for NextIconName and
PrevIconName currently document only the horizontal fallback icons, but
BitPivot’s runtime behavior also switches to vertical defaults. Update the
descriptions in BitPivotDemo.razor.cs so they mention both cases:
ChevronRight/ChevronLeft for horizontal pivots and ChevronDown/ChevronUp for
vertical pivots, matching the logic in BitPivot.razor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c7595f60-512b-431f-a90d-2660582bb96c
📒 Files selected for processing (8)
src/BlazorUI/Bit.BlazorUI/Components/Navs/Pivot/BitPivot.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Pivot/BitPivot.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Pivot/BitPivot.scsssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/PivotJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Pivot.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/Pivot/BitPivotDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/Pivot/BitPivotDemo.razor.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Pivot/BitPivotTests.cs
🚧 Files skipped from review as they are similar to previous changes (4)
- src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/PivotJsRuntimeExtensions.cs
- src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/Pivot/BitPivotDemo.razor
- src/BlazorUI/Bit.BlazorUI/Components/Navs/Pivot/BitPivot.razor
- src/BlazorUI/Bit.BlazorUI/Components/Navs/Pivot/BitPivot.razor.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #432
Summary by CodeRabbit
New Features
Bug Fixes
Tests