Apply BitMenuButton improvements (#12701)#12702
Merged
msynk merged 5 commits intoJul 20, 2026
Merged
Conversation
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (22)
WalkthroughBitMenuButton adds link and separator items, loading and full-width states, configurable drop direction, ARIA semantics, keyboard navigation, focus management, JavaScript lifecycle handling, expanded demos, and automated coverage. ChangesMenuButton improvements
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant BitMenuButton
participant MenuButtons
participant Callout
User->>BitMenuButton: Press keyboard navigation key
BitMenuButton->>MenuButtons: Request item focus
MenuButtons->>Callout: Select enabled item
Callout-->>User: Move focus to item
BitMenuButton-->>User: Restore trigger focus after close
Suggested reviewers: 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: 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/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor`:
- Around line 45-54: Update the anchor rendered by _BitMenuButtonItem to add
rel="noopener noreferrer" whenever MenuButton.GetTarget(Item) resolves to
"_blank"; leave rel unset for other targets and preserve the existing click,
URL, and styling behavior.
🪄 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: dd9d495a-d3cb-4835-81e3-e60b759f9026
📒 Files selected for processing (22)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.scsssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonItem.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonNameSelectors.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/MenuButtonsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/MenuButtons.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/BitMenuButtonDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/BitMenuButtonDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/Operation.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonCustomDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonCustomDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonCustomDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonItemDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonItemDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonItemDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonOptionDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonOptionDemo.razor.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/Buttons/MenuButton/BitMenuButton.razor`:
- Around line 92-101: Update the split-mode menu labeling logic in BitMenuButton
so the ul’s aria-labelledby references the operator button when
ChevronDownAriaLabel is unset, and uses the chevron button only when that label
provides an accessible name. Preserve the existing chevron target when an
explicit ChevronDownAriaLabel is supplied.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs`:
- Around line 884-897: Update OpenCallout to return immediately when IsLoading
is true, before changing focus state or calling AssignIsOpen. Keep the existing
opening and focus behavior unchanged when loading is false, so both
keyboard-triggered and split-chevron invocations respect the click-disable
contract.
In `@src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.scss`:
- Around line 34-50: Update the border-top-color value in the .bit-mnb-spn rule
from currentColor to the lowercase currentcolor keyword to satisfy the project's
stylelint value-keyword-case rule.
🪄 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: c8db6ba0-10c0-476b-83a3-9a682ca3ac3c
📒 Files selected for processing (22)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.scsssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonItem.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonNameSelectors.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/MenuButtonsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/MenuButtons.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/BitMenuButtonDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/BitMenuButtonDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/Operation.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonCustomDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonCustomDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonCustomDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonItemDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonItemDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonItemDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonOptionDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonOptionDemo.razor.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cs (1)
404-427: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider asserting
relfor thetarget="_blank"link item.The implementation now sets
rel="noopener noreferrer"whenTargetis"_blank"(fixing a past security review comment); adding an assertion here would lock in that regression protection.✅ Proposed addition
Assert.AreEqual("menuitem", anchor.GetAttribute("role")); + Assert.AreEqual("noopener noreferrer", anchor.GetAttribute("rel"));🤖 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/Buttons/MenuButton/BitMenuButtonTests.cs` around lines 404 - 427, Add an assertion in BitMenuButtonShouldRenderLinkItems for the rendered anchor’s rel attribute, verifying that a Target of "_blank" produces "noopener noreferrer" while preserving the existing link and button assertions.src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs (1)
405-416: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider guarding the first-render JS setup call against
JSDisconnectedException.
OnAfterRenderAsynccalls_js.BitMenuButtonsSetup(...)without a try/catch, whereasDisposeAsyncexplicitly ignoresJSDisconnectedExceptionfor the equivalent teardown call. If the circuit disconnects right around first render, this call could throw unhandled.🔧 Proposed fix
if (firstRender) { // Prevents the default behavior (scrolling) of the navigation keys handled by the // keydown handlers, since Blazor cannot conditionally preventDefault per key. - await _js.BitMenuButtonsSetup(_Id, _calloutId); + try + { + await _js.BitMenuButtonsSetup(_Id, _calloutId); + } + catch (JSDisconnectedException) { } // we can ignore this exception here }🤖 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/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs` around lines 405 - 416, Guard the first-render _js.BitMenuButtonsSetup call in OnAfterRenderAsync with a JSDisconnectedException catch, matching the existing disposal behavior. Ignore the disconnection exception while preserving normal propagation of other exceptions and leaving the base render call unchanged.
🤖 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/Buttons/MenuButton/BitMenuButton.razor.cs`:
- Around line 806-833: Update HandleOnTriggerKeyDown so ArrowDown/ArrowUp calls
FocusItem only after OpenCallout successfully opens the callout. Make
OpenCallout expose whether it opened, or otherwise verify the resulting open
state, while preserving the existing first/last item selection; ensure early
returns from its IsLoading guard or failed AssignIsOpen prevent focus.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs`:
- Around line 405-416: Guard the first-render _js.BitMenuButtonsSetup call in
OnAfterRenderAsync with a JSDisconnectedException catch, matching the existing
disposal behavior. Ignore the disconnection exception while preserving normal
propagation of other exceptions and leaving the base render call unchanged.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cs`:
- Around line 404-427: Add an assertion in BitMenuButtonShouldRenderLinkItems
for the rendered anchor’s rel attribute, verifying that a Target of "_blank"
produces "noopener noreferrer" while preserving the existing link and button
assertions.
🪄 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: 1af88e5a-9e0e-4049-b7ae-82b5559700df
📒 Files selected for processing (22)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.scsssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonItem.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonNameSelectors.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/MenuButtonsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/MenuButtons.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/BitMenuButtonDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/BitMenuButtonDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/Operation.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonCustomDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonCustomDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonCustomDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonItemDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonItemDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonItemDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonOptionDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/_BitMenuButtonOptionDemo.razor.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cs
…into 12701-blazorui-menubutton-improvements
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 28 minutes. |
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 #12701
Summary by CodeRabbit