fix(BTable): events are no longer dispatched when emitted from inside TRs and THs (regression of version 0.40.2) #2841
Conversation
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
|
|
Contributor
WalkthroughAdds early-return checks in BTableLite’s header and row keydown handlers to only process keys when focus is on TH or TR elements, respectively. Existing key actions (Enter/Space and navigation: ArrowUp/Down, Home, End) remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant DOM as DOM Element (TH/TR or other)
participant T as BTableLite.vue
U->>DOM: keydown
DOM->>T: handleHeaderKeydown / handleRowKeydown(event)
alt Target is focused and not expected tag (TH/TR)
T-->>DOM: return (no action)
note over T: New early-return guard
else Target is correct (TH/TR)
T->>T: Process key (Enter/Space, ArrowUp/Down, Home/End, Shift)
T-->>DOM: Apply selection/focus navigation
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches🧪 Generate unit tests
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 |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/bootstrap-vue-next/src/components/BTable/BTableLite.vue (2)
473-476: Prefer currentTarget check over tagName/activeElement; also bail if default preventedThis makes the intent explicit (“only handle when the event originates on this TH”), avoids brittle tagName checks, and is friendlier to slotted content and Shadow DOM.
- const {target, code} = event - - if (target && (target as Element).tagName !== 'TH' && document.activeElement === target) return + const {target, currentTarget, code, defaultPrevented} = event + if (defaultPrevented) return + if (target instanceof Element && currentTarget instanceof Element && target !== currentTarget) return
484-498: Apply the same safer check for rows; keep behavior identicalUse currentTarget to ignore bubbled keydowns from descendants and early-return if already handled elsewhere.
- const {target, code, shiftKey} = event - - if (target && (target as Element).tagName !== 'TR' && document.activeElement === target) return + const {target, currentTarget, code, shiftKey, defaultPrevented} = event + if (defaultPrevented) return + if (target instanceof Element && currentTarget instanceof Element && target !== currentTarget) return
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bootstrap-vue-next/src/components/BTable/BTableLite.vue(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/bootstrap-vue-next/src/components/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place all new Vue components under packages/bootstrap-vue-next/src/components/
Files:
packages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
packages/bootstrap-vue-next/src/components/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep component-specific styles inside the component .vue files
Files:
packages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
VividLemon
approved these changes
Sep 12, 2025
Merged
xvaara
added a commit
to xvaara/bootstrap-vue-next
that referenced
this pull request
Oct 11, 2025
* upstream/main: chore: release main (bootstrap-vue-next#2868) fix(useModalOrchestrator): circular dependency (bootstrap-vue-next#2874) docs(BModal): Parity pass (bootstrap-vue-next#2866) docs: Enable directly loading examples into StackBlitz (bootstrap-vue-next#2869) fix(BApp): wrap our test app in BApp in main.ts to enable easy verification of useModal, etc. (bootstrap-vue-next#2865) export useScrollLock() (bootstrap-vue-next#2854) chore: release main (bootstrap-vue-next#2858) fix(BToggle): stop looking for missing targets after directive is unmounted (bootstrap-vue-next#2857) chore: release main (bootstrap-vue-next#2851) Fix modal transition delays by making TransitionGroup name conditional (bootstrap-vue-next#2845) chore: release main (bootstrap-vue-next#2842) fix(BTable): events being wrongly stopped when sent from elements inside TRs (bootstrap-vue-next#2841)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Hello,
patch #2834 implemented keyboard navigation for BTable components, but unfortunately broke basic navigation (which previously worked).
Previously, it was possible to use the Tab key to focus elements (like buttons) in a BTable, and to use the spacebar or Enter key to activate these elements.
Now, when you use the spacebar or Enter key on these elements, either nothing happens or the TH/TR parent is incorrectly selected (when BTable has the "selectable" property).
This bug is caused by the fact that all keydown events are now handled by the BTr or BTh components and are systematically canceled, even though some keydown events should be handled by "internal" table elements.
My patch proposal is based on my own keyboard navigation implementation that I made some time ago (for my own use) and discussed here: https://stackblitz.com/edit/github-rzxbjv-nralwm3b
Maybe using
document.activeElementwouldn't even be necessary, but this patch works perfectly as is in my case.Small replication
I made 3 demos (each with the same 2 tables inside) so you can compare the results between different versions of boostrap-vue-next.
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