Apply BitDropdown improvements (#12741) - #12745
Open
msynk wants to merge 6 commits into
Open
Conversation
WalkthroughBitDropdown adds selection limits, select-all, empty/loading states, search modes, keyboard navigation, accessibility semantics, virtualization support, JavaScript focus integration, size styling, and expanded documentation and demos. ChangesBitDropdown improvements
Estimated code review effort: 4 (Complex) | ~60 minutes 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 |
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 8 minutes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/BlazorUI/Bit.BlazorUI/Scripts/Dropdowns.ts (1)
183-185: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePage step is a hard-coded 10 items.
Extract it as a named constant next to
VIRTUALIZE_RENDER_FRAMES, or derive it from the visible window (clientHeight / itemHeight) so PageUp/PageDown match what the user actually sees.🤖 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/Scripts/Dropdowns.ts` around lines 183 - 185, Replace the hard-coded 10-item page step in the nextPage and prevPage branches with a named constant declared alongside VIRTUALIZE_RENDER_FRAMES, or calculate the step from the visible window using clientHeight and itemHeight. Reuse that step for both navigation directions while preserving the existing boundary clamping behavior.src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor (1)
311-339: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSelect-all button stays focusable/clickable when the dropdown is disabled or read-only.
The
bit-drp-idsclass is applied forIsEnabled is false, but the<button>itself has nodisabledattribute, so it remains tab-reachable and is also collected by_getItemsinDropdowns.ts.ReadOnlyisn't reflected at all. The click handler bails out, so this is presentation/keyboard-focus only.♻️ Proposed change
<button `@onclick`="HandleOnSelectAllClick" type="button" role="checkbox" + disabled="@(IsEnabled is false || ReadOnly)" + aria-disabled="@(IsEnabled is false || ReadOnly)" style="`@Styles`?.SelectAllButton"🤖 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/Inputs/Dropdown/BitDropdown.razor` around lines 311 - 339, Update the select-all button rendered in the MultiSelect/ShowSelectAll block to be non-focusable and non-clickable when the dropdown is disabled or read-only. Reflect both IsEnabled and ReadOnly on the button’s disabled state so _getItems excludes it, while preserving its current enabled behavior and styling.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor (1)
707-707: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider
PreventRenderForMcpClienton the Virtualization example for consistency.The equivalent Virtualization example in
_BitDropdownItemDemo.razor(line 685) setsPreventRenderForMcpClient, while this one renders two 10k-item collections plus fourItemsProvider-backed dropdowns.🤖 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/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor` at line 707, Add PreventRenderForMcpClient to the Virtualization DemoExample identified by Id "example20" and title "Virtualization", matching the equivalent configuration in _BitDropdownItemDemo.razor.
🤖 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/Inputs/Dropdown/BitDropdown.razor`:
- Around line 341-346: Update the listbox markup in BitDropdown to conditionally
emit aria-labelledby using GetDropdownAriaLabelledby(), matching the trigger
behavior. Ensure no aria-labelledby attribute is rendered when neither
LabelTemplate nor Label creates the referenced label element.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 1448-1453: Update the callout keydown handling used by
HandleOnCalloutKeyDown and its Dropdowns.ts listener to prevent Tab’s default
browser focus movement, then close the callout and deliberately focus the
appropriate next or previous tabbable element based on Shift+Tab. Preserve
reachability of header/footer template controls and the search box in forward
tab order, rather than always refocusing _dropdownWrapperRef.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cs`:
- Around line 1742-1756: Update the sample Product class used by
example22CsharpCode and the corresponding class in the additional sample so it
declares the CssClass and CssStyle properties referenced by the nameSelectors
configuration; keep the existing selectors unchanged and ensure the copied
examples compile.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor`:
- Around line 311-339: Update the select-all button rendered in the
MultiSelect/ShowSelectAll block to be non-focusable and non-clickable when the
dropdown is disabled or read-only. Reflect both IsEnabled and ReadOnly on the
button’s disabled state so _getItems excludes it, while preserving its current
enabled behavior and styling.
In `@src/BlazorUI/Bit.BlazorUI/Scripts/Dropdowns.ts`:
- Around line 183-185: Replace the hard-coded 10-item page step in the nextPage
and prevPage branches with a named constant declared alongside
VIRTUALIZE_RENDER_FRAMES, or calculate the step from the visible window using
clientHeight and itemHeight. Reuse that step for both navigation directions
while preserving the existing boundary clamping behavior.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor`:
- Line 707: Add PreventRenderForMcpClient to the Virtualization DemoExample
identified by Id "example20" and title "Virtualization", matching the equivalent
configuration in _BitDropdownItemDemo.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 Plus
Run ID: 669d003b-b8c4-424c-b3d5-d5e8679f3ba6
📒 Files selected for processing (19)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/_BitDropdownItem.razorsrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/DropdownsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Dropdowns.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/BitDropdownDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/BitDropdownDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/BitDropdownDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownItemDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownItemDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownItemDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownOptionDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownOptionDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownOptionDemo.razor.samples.cs
….com/msynk/bitframework into 12741-blazorui-dropdown-improvements
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cs (1)
561-582: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSeveral new C# samples omit members their Razor sample references.
example9RazorCodeusescomboBoxCustomsandcomboBoxNameSelectors(lines 554-559), butexample9CsharpCodedefines neither, nor theProductclass. Same gap inexample16CsharpCode(razor usesnameSelectors,comboBoxCustoms,comboBoxNameSelectors),example22CsharpCode(razor usesnameSelectors) andexample26CsharpCode(noProduct). Sibling samples (example18,example19,example21) include the full boilerplate — worth aligning so the snippets are copy-pasteable.🤖 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/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cs` around lines 561 - 582, Complete the C# snippets in example9CsharpCode, example16CsharpCode, example22CsharpCode, and example26CsharpCode with every Product, collection, and selector member referenced by their corresponding Razor snippets, including comboBoxCustoms, comboBoxNameSelectors, nameSelectors, and Product as applicable. Align their declarations with the complete boilerplate used by example18, example19, and example21 so each sample is independently copy-pasteable.
🤖 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/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 860-883: Add a _positionsSelectionVersion backing field alongside
the existing position-cache fields, and include _selectionVersion in the cache
invalidation condition and cache-key updates within GetItemPositions(). This
must rebuild positions after selection changes, especially when
HideSelectedItems is enabled, while preserving the existing SearchText and
_optionsVersion cache behavior.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.scss`:
- Around line 1002-1008: Update the small dropdown styles in .bit-drp-sm so the
.bit-drp-pre/.bit-drp-suf height uses var(--bit-drp-h) instead of the fixed
spacing(3.75) value, allowing adorned controls to retain the configured small
height.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownItemDemo.razor.samples.cs`:
- Around line 841-847: Update the example16 sample’s backing declarations to
include the comboBoxItems list required by its BitDropdown Items binding,
matching the existing declaration pattern used in examples 9, 10, 14, 15, and
17. Ensure both the primary block and the additionally referenced range compile
without changing the sample’s other behavior.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownOptionDemo.razor.samples.cs`:
- Around line 1401-1454: Update the example22 C# sample code associated with
example22RazorCode to declare and initialize the basicItems collection before it
is used by the Razor loops and LoadDelayedItems. Keep the existing
isLoadingItems and delayedItems declarations unchanged so the sample remains
self-contained.
- Around line 1102-1112: Update the example16 sample code, including the related
example16CsharpCode block, to declare and initialize the comboBoxItems list
before the BitDropdown markup iterates it. Match the existing comboBoxItems
declaration pattern used by examples 14, 15, and 17 while preserving the
existing basicItems declaration and AutoClearSearch sample behavior.
---
Nitpick comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cs`:
- Around line 561-582: Complete the C# snippets in example9CsharpCode,
example16CsharpCode, example22CsharpCode, and example26CsharpCode with every
Product, collection, and selector member referenced by their corresponding Razor
snippets, including comboBoxCustoms, comboBoxNameSelectors, nameSelectors, and
Product as applicable. Align their declarations with the complete boilerplate
used by example18, example19, and example21 so each sample is independently
copy-pasteable.
🪄 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 Plus
Run ID: b79a8b8a-4de8-432f-a479-7002c7295fb6
📒 Files selected for processing (21)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownSearchMode.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/_BitDropdownItem.razorsrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/DropdownsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Dropdowns.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/BitDropdownDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/BitDropdownDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/BitDropdownDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownItemDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownItemDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownItemDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownOptionDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownOptionDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownOptionDemo.razor.samples.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownSearchMode.cs (1)
20-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Equalsas a member name hidesobject.Equalson the type.
BitDropdownSearchMode.Equals(a, b)no longer resolves to the staticobject.Equalsoverload, and the name reads oddly in an enum of match modes. Renaming later would be a breaking public API change, so considerExactMatch(orExact) now.🤖 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/Inputs/Dropdown/BitDropdownSearchMode.cs` around lines 20 - 23, Rename the BitDropdownSearchMode enum member Equals to a non-conflicting match-mode name such as ExactMatch, and update all references to the member accordingly. Preserve the existing exact text matching behavior while avoiding shadowing object.Equals and establishing the corrected public API name now.src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs (1)
1744-1750: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueFire-and-forget
ClearComboBoxInputnow reachesVirtualize.RefreshDataAsync.
ClearComboBoxInputawaitsSearchVirtualized(), so this discarded task performs JS/render work outside the caller's flow; any exception (e.g. anObjectDisposedExceptionwhen the component is torn down while the callout closes) is unobservable. Consider awaiting these from the caller ofOnSetIsOpenor at least logging the faulted task.🤖 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/Inputs/Dropdown/BitDropdown.razor.cs` around lines 1744 - 1750, Update the OnSetIsOpen flow to await ClearComboBoxInput instead of discarding its task, allowing the SearchVirtualized and Virtualize.RefreshDataAsync work and exceptions to remain in the caller’s async flow. Preserve the existing ordering so the combo input is cleared before invoking OnOpen or OnClose.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cs (1)
626-628: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCombo sample
TextSetterlambdas declare a nullable first parameter.BitDropdownNameSelectors.TextSetterisAction<string, TItem>?and the real code-behind uses(string text, Product item); the samples all usestring?, so copied code warns on nullability.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cs#L626-L628: change(string? text, Product item)to(string text, Product item)inexample9CsharpCode..../_BitDropdownCustomDemo.razor.samples.cs#L1153-L1155: same change inexample14CsharpCode..../_BitDropdownCustomDemo.razor.samples.cs#L1222-L1224: same change inexample15CsharpCode..../_BitDropdownCustomDemo.razor.samples.cs#L1342-L1344: same change inexample16CsharpCode..../_BitDropdownCustomDemo.razor.samples.cs#L1431-L1433: same change inexample17CsharpCode.🤖 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/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cs` around lines 626 - 628, Update the TextSetter lambda parameter type from nullable string? to non-nullable string in example9CsharpCode at src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cs:626-628, and apply the same change in example14CsharpCode at :1153-1155, example15CsharpCode at :1222-1224, example16CsharpCode at :1342-1344, and example17CsharpCode at :1431-1433. Keep the Product item parameter and setter behavior 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/Inputs/Dropdown/BitDropdown.razor`:
- Around line 466-473: Keep the live-region container associated with
GetSearchResultsText permanently mounted in the dropdown markup, and
conditionally vary or clear only its text based on HasSearchText and IsLoading.
Preserve the existing polite, atomic status semantics while ensuring the element
remains in the accessibility tree before search-result announcements occur.
In `@src/BlazorUI/Bit.BlazorUI/Scripts/Dropdowns.ts`:
- Around line 116-122: Update the visibility filter in BitDropdowns._getItems to
remove the offsetParent check, which excludes valid position-fixed dropdown
items. Replace it with a robust visibility check using checkVisibility() or
nonzero getBoundingClientRect() dimensions while preserving the existing
disabled and excluded-container filters.
- Around line 85-170: Serialize virtualized focus operations per callout by
adding a generation token associated with each callout and incrementing it at
the start of focusItem. Capture the token before awaiting _scrollFor, then abort
the stale invocation if the token changes after the await, preventing outdated
calls from applying interleaved items, scrolling, or focus results.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownOptionDemo.razor.samples.cs`:
- Around line 488-512: Update the example9 sample declarations to include the
comboBoxItems collection referenced by the “Highlight in the ComboBox” markup,
matching the equivalent declarations in _BitDropdownItemDemo.razor.samples.cs
while retaining basicItems for the existing dropdown examples.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 1744-1750: Update the OnSetIsOpen flow to await ClearComboBoxInput
instead of discarding its task, allowing the SearchVirtualized and
Virtualize.RefreshDataAsync work and exceptions to remain in the caller’s async
flow. Preserve the existing ordering so the combo input is cleared before
invoking OnOpen or OnClose.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownSearchMode.cs`:
- Around line 20-23: Rename the BitDropdownSearchMode enum member Equals to a
non-conflicting match-mode name such as ExactMatch, and update all references to
the member accordingly. Preserve the existing exact text matching behavior while
avoiding shadowing object.Equals and establishing the corrected public API name
now.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cs`:
- Around line 626-628: Update the TextSetter lambda parameter type from nullable
string? to non-nullable string in example9CsharpCode at
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cs:626-628,
and apply the same change in example14CsharpCode at :1153-1155,
example15CsharpCode at :1222-1224, example16CsharpCode at :1342-1344, and
example17CsharpCode at :1431-1433. Keep the Product item parameter and setter
behavior unchanged.
🪄 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 Plus
Run ID: 91a1b8ab-ff01-4c43-b4d6-289c5d006623
📒 Files selected for processing (21)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownSearchMode.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/_BitDropdownItem.razorsrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/DropdownsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Dropdowns.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/BitDropdownDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/BitDropdownDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/BitDropdownDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownCustomDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownItemDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownItemDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownItemDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownOptionDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownOptionDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownOptionDemo.razor.samples.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownTests.cs
Comment on lines
+466
to
+473
| @if (HasSearchText && IsLoading is false) | ||
| { | ||
| @* A search silently changes the length of a list a screen reader user cannot see, so the new | ||
| result count is announced. Only rendered while searching, to stay quiet otherwise. *@ | ||
| <div role="status" aria-live="polite" aria-atomic="true" class="bit-drp-lvr"> | ||
| @GetSearchResultsText() | ||
| </div> | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Live region is inserted together with its text, so it may never be announced.
Most screen readers only announce mutations of a live region that already existed in the accessibility tree. Since the whole role="status" element appears on the first keystroke and disappears when the search is cleared, the initial (and final) announcement is likely to be dropped. Keep the container mounted and vary only its content.
♿ Proposed fix
- `@if` (HasSearchText && IsLoading is false)
- {
- @* A search silently changes the length of a list a screen reader user cannot see, so the new
- result count is announced. Only rendered while searching, to stay quiet otherwise. *@
- <div role="status" aria-live="polite" aria-atomic="true" class="bit-drp-lvr">
- `@GetSearchResultsText`()
- </div>
- }
+ @* A search silently changes the length of a list a screen reader user cannot see, so the new
+ result count is announced. The region itself stays mounted (a live region added together with
+ its text is usually not announced) and is simply empty while there is nothing to report. *@
+ <div role="status" aria-live="polite" aria-atomic="true" class="bit-drp-lvr">
+ @(HasSearchText && IsLoading is false ? GetSearchResultsText() : null)
+ </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @if (HasSearchText && IsLoading is false) | |
| { | |
| @* A search silently changes the length of a list a screen reader user cannot see, so the new | |
| result count is announced. Only rendered while searching, to stay quiet otherwise. *@ | |
| <div role="status" aria-live="polite" aria-atomic="true" class="bit-drp-lvr"> | |
| @GetSearchResultsText() | |
| </div> | |
| } | |
| @* A search silently changes the length of a list a screen reader user cannot see, so the new | |
| result count is announced. The region itself stays mounted (a live region added together with | |
| its text is usually not announced) and is simply empty while there is nothing to report. *@ | |
| <div role="status" aria-live="polite" aria-atomic="true" class="bit-drp-lvr"> | |
| @(HasSearchText && IsLoading is false ? GetSearchResultsText() : null) | |
| </div> |
🤖 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/Inputs/Dropdown/BitDropdown.razor`
around lines 466 - 473, Keep the live-region container associated with
GetSearchResultsText permanently mounted in the dropdown markup, and
conditionally vary or clear only its text based on HasSearchText and IsLoading.
Preserve the existing polite, atomic status semantics while ensuring the element
remains in the accessibility tree before search-result announcements occur.
Comment on lines
+85
to
+170
| public static async focusItem(calloutId: string, mode: string, char: string | null, virtualize: boolean) { | ||
| const callout = document.getElementById(calloutId); | ||
| if (!callout) return; | ||
|
|
||
| const scroller = callout.querySelector('.bit-drp-scn') as HTMLElement | null; | ||
|
|
||
| let items = Dropdowns._getItems(callout); | ||
| if (items.length === 0) return; | ||
|
|
||
| let current = items.indexOf(document.activeElement as HTMLElement); | ||
|
|
||
| // In virtualize mode only the items around the visible window exist in the DOM, so moving | ||
| // beyond it means scrolling first and continuing with the items rendered afterwards. | ||
| if (virtualize && Dropdowns._isScrollable(scroller)) { | ||
| const scrolled = await Dropdowns._scrollFor(callout, scroller!, mode, items); | ||
| if (scrolled) { | ||
| items = scrolled.items; | ||
| current = scrolled.items.indexOf(document.activeElement as HTMLElement); | ||
| mode = scrolled.mode; | ||
| } | ||
| } | ||
|
|
||
| const index = Dropdowns._resolveIndex(items, current, mode, char, virtualize); | ||
|
|
||
| if (index > -1) { | ||
| items[index].focus(); | ||
| } | ||
| } | ||
|
|
||
| // The options plus the select all item, which is not an option (it is a checkbox outside of the | ||
| // listbox) but still has to be reachable with the arrow keys. | ||
| private static _getItems(callout: HTMLElement) { | ||
| return (Array.from(callout.querySelectorAll('[role="option"], .bit-drp-sab')) as HTMLElement[]) | ||
| .filter(el => !(el as HTMLButtonElement).disabled && | ||
| el.getAttribute('aria-disabled') !== 'true' && | ||
| !el.closest('.bit-drp-ids') && | ||
| el.offsetParent !== null); | ||
| } | ||
|
|
||
| private static _isScrollable(scroller: HTMLElement | null) { | ||
| return !!scroller && scroller.scrollHeight > scroller.clientHeight + 1; | ||
| } | ||
|
|
||
| // Scrolls the item container for the jump modes and resolves once the items revealed by the | ||
| // scroll have been rendered, mapping the mode to where the focus goes in the new window. | ||
| private static async _scrollFor(callout: HTMLElement, scroller: HTMLElement, mode: string, items: HTMLElement[]) { | ||
| const max = scroller.scrollHeight - scroller.clientHeight; | ||
|
|
||
| let top: number; | ||
| let nextMode = mode; | ||
|
|
||
| if (mode === 'first') { | ||
| top = 0; | ||
| } else if (mode === 'last') { | ||
| top = max; | ||
| } else if (mode === 'prevPage') { | ||
| top = scroller.scrollTop - scroller.clientHeight; | ||
| nextMode = 'first'; | ||
| } else if (mode === 'nextPage') { | ||
| top = scroller.scrollTop + scroller.clientHeight; | ||
| nextMode = 'last'; | ||
| } else { | ||
| // The arrow keys move one item at a time, which scrolls the focused item into view and | ||
| // makes virtualization render the next ones on its own. | ||
| return null; | ||
| } | ||
|
|
||
| top = Math.max(0, Math.min(max, top)); | ||
| if (top === scroller.scrollTop) return null; | ||
|
|
||
| const first = items[0]; | ||
| const last = items[items.length - 1]; | ||
|
|
||
| scroller.scrollTop = top; | ||
|
|
||
| for (let i = 0; i < Dropdowns.VIRTUALIZE_RENDER_FRAMES; i++) { | ||
| await new Promise<void>(resolve => requestAnimationFrame(() => resolve())); | ||
|
|
||
| const rendered = Dropdowns._getItems(callout); | ||
| if (rendered.length > 0 && (rendered[0] !== first || rendered[rendered.length - 1] !== last)) { | ||
| return { items: rendered, mode: nextMode }; | ||
| } | ||
| } | ||
|
|
||
| return { items: Dropdowns._getItems(callout), mode: nextMode }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Overlapping focusItem calls can race in virtualize mode.
_scrollFor awaits up to VIRTUALIZE_RENDER_FRAMES animation frames while polling for newly rendered items. Holding down an arrow key (or fast repeated presses) fires keydown events faster than this resolves, so multiple focusItem invocations can run concurrently, independently mutating scroller.scrollTop and computing items/current from interleaved DOM snapshots. The result can be an incorrect scroll position or the wrong item receiving focus.
Consider tracking a per-callout generation token, incrementing it at the start of focusItem, and bailing out if the token changes while awaiting _scrollFor.
💡 Suggested fix sketch (generation guard)
+ private static _generation = new Map<string, number>();
+
public static async focusItem(calloutId: string, mode: string, char: string | null, virtualize: boolean) {
const callout = document.getElementById(calloutId);
if (!callout) return;
+ const gen = (Dropdowns._generation.get(calloutId) ?? 0) + 1;
+ Dropdowns._generation.set(calloutId, gen);
+
const scroller = callout.querySelector('.bit-drp-scn') as HTMLElement | null;
let items = Dropdowns._getItems(callout);
if (items.length === 0) return;
let current = items.indexOf(document.activeElement as HTMLElement);
if (virtualize && Dropdowns._isScrollable(scroller)) {
const scrolled = await Dropdowns._scrollFor(callout, scroller!, mode, items);
+ if (Dropdowns._generation.get(calloutId) !== gen) return; // stale
if (scrolled) {
items = scrolled.items;
current = scrolled.items.indexOf(document.activeElement as HTMLElement);
mode = scrolled.mode;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public static async focusItem(calloutId: string, mode: string, char: string | null, virtualize: boolean) { | |
| const callout = document.getElementById(calloutId); | |
| if (!callout) return; | |
| const scroller = callout.querySelector('.bit-drp-scn') as HTMLElement | null; | |
| let items = Dropdowns._getItems(callout); | |
| if (items.length === 0) return; | |
| let current = items.indexOf(document.activeElement as HTMLElement); | |
| // In virtualize mode only the items around the visible window exist in the DOM, so moving | |
| // beyond it means scrolling first and continuing with the items rendered afterwards. | |
| if (virtualize && Dropdowns._isScrollable(scroller)) { | |
| const scrolled = await Dropdowns._scrollFor(callout, scroller!, mode, items); | |
| if (scrolled) { | |
| items = scrolled.items; | |
| current = scrolled.items.indexOf(document.activeElement as HTMLElement); | |
| mode = scrolled.mode; | |
| } | |
| } | |
| const index = Dropdowns._resolveIndex(items, current, mode, char, virtualize); | |
| if (index > -1) { | |
| items[index].focus(); | |
| } | |
| } | |
| // The options plus the select all item, which is not an option (it is a checkbox outside of the | |
| // listbox) but still has to be reachable with the arrow keys. | |
| private static _getItems(callout: HTMLElement) { | |
| return (Array.from(callout.querySelectorAll('[role="option"], .bit-drp-sab')) as HTMLElement[]) | |
| .filter(el => !(el as HTMLButtonElement).disabled && | |
| el.getAttribute('aria-disabled') !== 'true' && | |
| !el.closest('.bit-drp-ids') && | |
| el.offsetParent !== null); | |
| } | |
| private static _isScrollable(scroller: HTMLElement | null) { | |
| return !!scroller && scroller.scrollHeight > scroller.clientHeight + 1; | |
| } | |
| // Scrolls the item container for the jump modes and resolves once the items revealed by the | |
| // scroll have been rendered, mapping the mode to where the focus goes in the new window. | |
| private static async _scrollFor(callout: HTMLElement, scroller: HTMLElement, mode: string, items: HTMLElement[]) { | |
| const max = scroller.scrollHeight - scroller.clientHeight; | |
| let top: number; | |
| let nextMode = mode; | |
| if (mode === 'first') { | |
| top = 0; | |
| } else if (mode === 'last') { | |
| top = max; | |
| } else if (mode === 'prevPage') { | |
| top = scroller.scrollTop - scroller.clientHeight; | |
| nextMode = 'first'; | |
| } else if (mode === 'nextPage') { | |
| top = scroller.scrollTop + scroller.clientHeight; | |
| nextMode = 'last'; | |
| } else { | |
| // The arrow keys move one item at a time, which scrolls the focused item into view and | |
| // makes virtualization render the next ones on its own. | |
| return null; | |
| } | |
| top = Math.max(0, Math.min(max, top)); | |
| if (top === scroller.scrollTop) return null; | |
| const first = items[0]; | |
| const last = items[items.length - 1]; | |
| scroller.scrollTop = top; | |
| for (let i = 0; i < Dropdowns.VIRTUALIZE_RENDER_FRAMES; i++) { | |
| await new Promise<void>(resolve => requestAnimationFrame(() => resolve())); | |
| const rendered = Dropdowns._getItems(callout); | |
| if (rendered.length > 0 && (rendered[0] !== first || rendered[rendered.length - 1] !== last)) { | |
| return { items: rendered, mode: nextMode }; | |
| } | |
| } | |
| return { items: Dropdowns._getItems(callout), mode: nextMode }; | |
| } | |
| private static _generation = new Map<string, number>(); | |
| public static async focusItem(calloutId: string, mode: string, char: string | null, virtualize: boolean) { | |
| const callout = document.getElementById(calloutId); | |
| if (!callout) return; | |
| const gen = (Dropdowns._generation.get(calloutId) ?? 0) + 1; | |
| Dropdowns._generation.set(calloutId, gen); | |
| const scroller = callout.querySelector('.bit-drp-scn') as HTMLElement | null; | |
| let items = Dropdowns._getItems(callout); | |
| if (items.length === 0) return; | |
| let current = items.indexOf(document.activeElement as HTMLElement); | |
| // In virtualize mode only the items around the visible window exist in the DOM, so moving | |
| // beyond it means scrolling first and continuing with the items rendered afterwards. | |
| if (virtualize && Dropdowns._isScrollable(scroller)) { | |
| const scrolled = await Dropdowns._scrollFor(callout, scroller!, mode, items); | |
| if (Dropdowns._generation.get(calloutId) !== gen) return; | |
| if (scrolled) { | |
| items = scrolled.items; | |
| current = scrolled.items.indexOf(document.activeElement as HTMLElement); | |
| mode = scrolled.mode; | |
| } | |
| } | |
| const index = Dropdowns._resolveIndex(items, current, mode, char, virtualize); | |
| if (index > -1) { | |
| items[index].focus(); | |
| } | |
| } | |
| // The options plus the select all item, which is not an option (it is a checkbox outside of the | |
| // listbox) but still has to be reachable with the arrow keys. | |
| private static _getItems(callout: HTMLElement) { | |
| return (Array.from(callout.querySelectorAll('[role="option"], .bit-drp-sab')) as HTMLElement[]) | |
| .filter(el => !(el as HTMLButtonElement).disabled && | |
| el.getAttribute('aria-disabled') !== 'true' && | |
| !el.closest('.bit-drp-ids') && | |
| el.offsetParent !== null); | |
| } | |
| private static _isScrollable(scroller: HTMLElement | null) { | |
| return !!scroller && scroller.scrollHeight > scroller.clientHeight + 1; | |
| } | |
| // Scrolls the item container for the jump modes and resolves once the items revealed by the | |
| // scroll have been rendered, mapping the mode to where the focus goes in the new window. | |
| private static async _scrollFor(callout: HTMLElement, scroller: HTMLElement, mode: string, items: HTMLElement[]) { | |
| const max = scroller.scrollHeight - scroller.clientHeight; | |
| let top: number; | |
| let nextMode = mode; | |
| if (mode === 'first') { | |
| top = 0; | |
| } else if (mode === 'last') { | |
| top = max; | |
| } else if (mode === 'prevPage') { | |
| top = scroller.scrollTop - scroller.clientHeight; | |
| nextMode = 'first'; | |
| } else if (mode === 'nextPage') { | |
| top = scroller.scrollTop + scroller.clientHeight; | |
| nextMode = 'last'; | |
| } else { | |
| // The arrow keys move one item at a time, which scrolls the focused item into view and | |
| // makes virtualization render the next ones on its own. | |
| return null; | |
| } | |
| top = Math.max(0, Math.min(max, top)); | |
| if (top === scroller.scrollTop) return null; | |
| const first = items[0]; | |
| const last = items[items.length - 1]; | |
| scroller.scrollTop = top; | |
| for (let i = 0; i < Dropdowns.VIRTUALIZE_RENDER_FRAMES; i++) { | |
| await new Promise<void>(resolve => requestAnimationFrame(() => resolve())); | |
| const rendered = Dropdowns._getItems(callout); | |
| if (rendered.length > 0 && (rendered[0] !== first || rendered[rendered.length - 1] !== last)) { | |
| return { items: rendered, mode: nextMode }; | |
| } | |
| } | |
| return { items: Dropdowns._getItems(callout), mode: nextMode }; | |
| } |
🤖 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/Scripts/Dropdowns.ts` around lines 85 - 170,
Serialize virtualized focus operations per callout by adding a generation token
associated with each callout and incrementing it at the start of focusItem.
Capture the token before awaiting _scrollFor, then abort the stale invocation if
the token changes after the await, preventing outdated calls from applying
interleaved items, scrolling, or focus results.
Comment on lines
+116
to
+122
| private static _getItems(callout: HTMLElement) { | ||
| return (Array.from(callout.querySelectorAll('[role="option"], .bit-drp-sab')) as HTMLElement[]) | ||
| .filter(el => !(el as HTMLButtonElement).disabled && | ||
| el.getAttribute('aria-disabled') !== 'true' && | ||
| !el.closest('.bit-drp-ids') && | ||
| el.offsetParent !== null); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the dropdown/callout uses position: fixed anywhere in its styling
rg -n -i 'position\s*:\s*fixed' src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/
rg -n -i 'position\s*:\s*fixed' src/BlazorUI/Bit.BlazorUI/Components/Callout* -g '*.scss' 2>/dev/nullRepository: bitfoundation/bitplatform
Length of output: 353
🏁 Script executed:
#!/bin/bash
set -e
echo "== Dropdown SCSS fixed positioning context =="
sed -n '620,700p' src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.scss
echo
echo "== Dropdown.ts relevant lines =="
wc -l src/BlazorUI/Bit.BlazorUI/Scripts/Dropdowns.ts
sed -n '90,155p' src/BlazorUI/Bit.BlazorUI/Scripts/Dropdowns.ts
echo
echo "== Find Dropdown.ts references to _getItems/focusItem =="
rg -n "_getItems|focusItem|offsetParent|bit-drp-sab|role=\"option\"" src/BlazorUI/Bit.BlazorUI/Scripts/Dropdowns.ts
echo
echo "== Nearby callout fixed references in scss =="
rg -n -i -C 3 'position\s*:\s*fixed' src/BlazorUI/Bit.BlazorUI/Components/Callout* -g '*.scss' 2>/dev/null || trueRepository: bitfoundation/bitplatform
Length of output: 5709
Avoid using offsetParent to filter dropdown items.
BitDropdown.scss uses position: fixed for the callout, and HTMLElement.offsetParent is null for fixed elements, not only for hidden ones. This can filter out valid items and cause focusItem() to exit before navigating arrow/type-ahead focus targets. Use a robust visibility check such as checkVisibility() or getBoundingClientRect() dimensions instead.
🤖 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/Scripts/Dropdowns.ts` around lines 116 - 122,
Update the visibility filter in BitDropdowns._getItems to remove the
offsetParent check, which excludes valid position-fixed dropdown items. Replace
it with a robust visibility check using checkVisibility() or nonzero
getBoundingClientRect() dimensions while preserving the existing disabled and
excluded-container filters.
Comment on lines
+488
to
+512
| <BitDropdown Label=""Highlight in the ComboBox"" | ||
| Combo | ||
| HighlightSearch | ||
| DefaultValue=""@string.Empty"" | ||
| Placeholder=""Type to filter"" | ||
| TItem=""BitDropdownOption<string>"" TValue=""string""> | ||
| @foreach (var item in comboBoxItems) | ||
| { | ||
| <BitDropdownOption ItemType=""item.ItemType"" Text=""@item.Text"" Value=""item.Value"" IsEnabled=""item.IsEnabled"" /> | ||
| } | ||
| </BitDropdown>"; | ||
| private readonly string example9CsharpCode = @" | ||
| private readonly List<BitDropdownItem<string>> basicItems = | ||
| [ | ||
| new() { ItemType = BitDropdownItemType.Header, Text = ""Fruits"" }, | ||
| new() { Text = ""Apple"", Value = ""f-app"" }, | ||
| new() { Text = ""Banana"", Value = ""f-ban"" }, | ||
| new() { Text = ""Orange"", Value = ""f-ora"", IsEnabled = false }, | ||
| new() { Text = ""Grape"", Value = ""f-gra"" }, | ||
| new() { ItemType = BitDropdownItemType.Divider }, | ||
| new() { ItemType = BitDropdownItemType.Header, Text = ""Vegetables"" }, | ||
| new() { Text = ""Broccoli"", Value = ""v-bro"" }, | ||
| new() { Text = ""Carrot"", Value = ""v-car"" }, | ||
| new() { Text = ""Lettuce"", Value = ""v-let"" } | ||
| ];"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
example9 sample iterates comboBoxItems but only declares basicItems.
The "Highlight in the ComboBox" block (line 494) uses comboBoxItems, yet example9CsharpCode declares just basicItems, so the snippet isn't copy-pasteable. The equivalent sample in _BitDropdownItemDemo.razor.samples.cs declares both.
📝 Proposed fix
private readonly string example9CsharpCode = @"
+private readonly List<BitDropdownItem<string>> comboBoxItems =
+[
+ new() { ItemType = BitDropdownItemType.Header, Text = ""Fruits"" },
+ new() { Text = ""Apple"", Value = ""f-app"" },
+ new() { Text = ""Banana"", Value = ""f-ban"" },
+ new() { Text = ""Orange"", Value = ""f-ora"", IsEnabled = false },
+ new() { Text = ""Grape"", Value = ""f-gra"" },
+ new() { ItemType = BitDropdownItemType.Divider },
+ new() { ItemType = BitDropdownItemType.Header, Text = ""Vegetables"" },
+ new() { Text = ""Broccoli"", Value = ""v-bro"" },
+ new() { Text = ""Carrot"", Value = ""v-car"" },
+ new() { Text = ""Lettuce"", Value = ""v-let"" }
+];
+
private readonly List<BitDropdownItem<string>> basicItems =📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <BitDropdown Label=""Highlight in the ComboBox"" | |
| Combo | |
| HighlightSearch | |
| DefaultValue=""@string.Empty"" | |
| Placeholder=""Type to filter"" | |
| TItem=""BitDropdownOption<string>"" TValue=""string""> | |
| @foreach (var item in comboBoxItems) | |
| { | |
| <BitDropdownOption ItemType=""item.ItemType"" Text=""@item.Text"" Value=""item.Value"" IsEnabled=""item.IsEnabled"" /> | |
| } | |
| </BitDropdown>"; | |
| private readonly string example9CsharpCode = @" | |
| private readonly List<BitDropdownItem<string>> basicItems = | |
| [ | |
| new() { ItemType = BitDropdownItemType.Header, Text = ""Fruits"" }, | |
| new() { Text = ""Apple"", Value = ""f-app"" }, | |
| new() { Text = ""Banana"", Value = ""f-ban"" }, | |
| new() { Text = ""Orange"", Value = ""f-ora"", IsEnabled = false }, | |
| new() { Text = ""Grape"", Value = ""f-gra"" }, | |
| new() { ItemType = BitDropdownItemType.Divider }, | |
| new() { ItemType = BitDropdownItemType.Header, Text = ""Vegetables"" }, | |
| new() { Text = ""Broccoli"", Value = ""v-bro"" }, | |
| new() { Text = ""Carrot"", Value = ""v-car"" }, | |
| new() { Text = ""Lettuce"", Value = ""v-let"" } | |
| ];"; | |
| private readonly string example9CsharpCode = @" | |
| private readonly List<BitDropdownItem<string>> comboBoxItems = | |
| [ | |
| new() { ItemType = BitDropdownItemType.Header, Text = ""Fruits"" }, | |
| new() { Text = ""Apple"", Value = ""f-app"" }, | |
| new() { Text = ""Banana"", Value = ""f-ban"" }, | |
| new() { Text = ""Orange"", Value = ""f-ora"", IsEnabled = false }, | |
| new() { Text = ""Grape"", Value = ""f-gra"" }, | |
| new() { ItemType = BitDropdownItemType.Divider }, | |
| new() { ItemType = BitDropdownItemType.Header, Text = ""Vegetables"" }, | |
| new() { Text = ""Broccoli"", Value = ""v-bro"" }, | |
| new() { Text = ""Carrot"", Value = ""v-car"" }, | |
| new() { Text = ""Lettuce"", Value = ""v-let"" } | |
| ]; | |
| private readonly List<BitDropdownItem<string>> basicItems = | |
| [ | |
| new() { ItemType = BitDropdownItemType.Header, Text = ""Fruits"" }, | |
| new() { Text = ""Apple"", Value = ""f-app"" }, | |
| new() { Text = ""Banana"", Value = ""f-ban"" }, | |
| new() { Text = ""Orange"", Value = ""f-ora"", IsEnabled = false }, | |
| new() { Text = ""Grape"", Value = ""f-gra"" }, | |
| new() { ItemType = BitDropdownItemType.Divider }, | |
| new() { ItemType = BitDropdownItemType.Header, Text = ""Vegetables"" }, | |
| new() { Text = ""Broccoli"", Value = ""v-bro"" }, | |
| new() { Text = ""Carrot"", Value = ""v-car"" }, | |
| new() { Text = ""Lettuce"", Value = ""v-let"" } | |
| ];"; |
🤖 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/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/Dropdown/_BitDropdownOptionDemo.razor.samples.cs`
around lines 488 - 512, Update the example9 sample declarations to include the
comboBoxItems collection referenced by the “Highlight in the ComboBox” markup,
matching the equivalent declarations in _BitDropdownItemDemo.razor.samples.cs
while retaining basicItems for the existing dropdown examples.
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 #12741
Summary by CodeRabbit