Conversation
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
CHANGELOGNo commits contributing to a release was found, no changelog entries will be added for this Pull Request. If this PR should be included in a release amend the commit message(s) to use:
Read more about the release process (swedish). |
Artifact sizesArtifact sizes in this build (unchanged artifacts collapsed below).
7 unchanged artifacts
|
Förhandsgranskning 🐛 🔍Dokumentation och exampel applikationer finns att förhandsgranska på: Senast uppdaterad 2026-03-09 12:16 UTC i gh-pages. |
WalkthroughThe changes refactor event handling across FTable row and header components from input change events to explicit click handlers on both table cells (td) and input elements. This introduces cell activation via the activateCell utility and adds event propagation control. The modifications affect row selection (anchor, checkbox, radio), expansion buttons, buttons, and header selectable controls. Corresponding test updates change interaction simulation from setValue() to trigger("click"). No public API signatures are altered. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vue-labs/src/components/FTable/FTable.spec.ts`:
- Line 866: The test currently triggers clicks on the nested input ("tbody
tr:first-child input") but lacks coverage for the new cell/header click proxies;
add test cases that click the containing cell/header elements (e.g., trigger
click on the containing "td" and on the header "th") and assert that the
selection change handler is invoked exactly once per click (use the existing
selection spy or emitted event used elsewhere in this spec). Update the two
locations noted (around the existing input click at the first case and the one
at line ~898) to include a cell click and a header click with precise assertions
like toHaveBeenCalledTimes(1) or the equivalent emitted check so the
clickable-cell proxy path is covered.
In `@packages/vue-labs/src/components/FTable/ITableAnchor.vue`:
- Around line 21-24: The onClickTd function programmatically clicks the proxied
anchor (targetElement.value.click()) which can bubble back into the same td
click handler and cause re-entry or double navigation; introduce a short-lived
guard flag (e.g., isProgrammaticClick as a ref/boolean) referenced by the td
click handler and by onClickTd: set the flag true immediately before calling
targetElement.value.click(), have the td click handler return early if the flag
is true, and clear the flag after the programmatic click completes (use nextTick
or a microtask/setTimeout to reset it) so regular user clicks still work. Ensure
you update both onClickTd and the td click handler that currently processes
clicks on the cell.
In `@packages/vue-labs/src/components/FTable/ITableCheckbox.vue`:
- Around line 23-35: onClickTd currently calls column.update(row,
!checked.value, checked.value) which reverses the argument order used in
onClickInput (column.update(row, checked.value, !checked.value)); change
onClickTd to call column.update(row, checked.value, !checked.value) so td clicks
request the same state transition as input clicks, and apply the same correction
to the other identical handler referenced around lines 42-43; keep the existing
e.stopPropagation, assertRef(targetElement), and
activateCell(targetElement.value, { focus: true }) calls unchanged.
In `@packages/vue-labs/src/components/FTable/ITableHeaderSelectable.vue`:
- Around line 30-35: The onClick handler (function onClick) must early-return
when the bulk checkbox/input doesn't exist to avoid calling assertRef on a
missing ref; update onClick to check that inputRef and inputRef.value exist (or
check the selection mode is "multi") before calling assertRef(inputRef),
activateCell(...), and emit("toggle"), and simply return when the input is
absent so the single-select header won't throw.
In `@packages/vue-labs/src/components/FTable/ITableRadio.vue`:
- Around line 23-35: The td click handler onClickTd is calling column.update
with the arguments flipped compared to onClickInput, causing a different
transition; change onClickTd to call column.update(row, checked.value,
!checked.value) (same as onClickInput) after activateCell, and also fix the
other similar handler later (the other td/cell click at lines ~42-43) so all td
click paths use the same column.update(row, checked.value, !checked.value)
signature; ensure you still call assertRef(inputElement) and
activateCell(inputElement.value, { focus: true }) before the update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: de30852c-cc83-4de7-ac30-7e6860e0cbc5
📒 Files selected for processing (7)
packages/vue-labs/src/components/FTable/FTable.spec.tspackages/vue-labs/src/components/FTable/ITableAnchor.vuepackages/vue-labs/src/components/FTable/ITableButton.vuepackages/vue-labs/src/components/FTable/ITableCheckbox.vuepackages/vue-labs/src/components/FTable/ITableExpandButton.vuepackages/vue-labs/src/components/FTable/ITableHeaderSelectable.vuepackages/vue-labs/src/components/FTable/ITableRadio.vue
| ).toBe("false"); | ||
|
|
||
| await wrapper.get("tbody tr:first-child input").setValue(); | ||
| await wrapper.get("tbody tr:first-child input").trigger("click"); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add coverage for the new td/th click proxies.
These assertions now click the nested inputs, but the risky behavior added in this PR is the new cell/header proxy handlers. Please add cases that click the containing td/th and assert exactly one selection change, otherwise regressions in the clickable-cell path will slip through.
Also applies to: 898-898
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vue-labs/src/components/FTable/FTable.spec.ts` at line 866, The test
currently triggers clicks on the nested input ("tbody tr:first-child input") but
lacks coverage for the new cell/header click proxies; add test cases that click
the containing cell/header elements (e.g., trigger click on the containing "td"
and on the header "th") and assert that the selection change handler is invoked
exactly once per click (use the existing selection spy or emitted event used
elsewhere in this spec). Update the two locations noted (around the existing
input click at the first case and the one at line ~898) to include a cell click
and a header click with precise assertions like toHaveBeenCalledTimes(1) or the
equivalent emitted check so the clickable-cell proxy path is covered.
MCFK
force-pushed
the
feat/clickable-cells
branch
from
e530cc1 to
c5228a2
Compare
MCFK
force-pushed
the
feat/clickable-cells
branch
from
c5228a2 to
e07c4f7
Compare
|
@coderabbitai generate unit tests |
|
Request timed out after 900000ms (requestId=1196f72a-f71d-41e1-95b8-376457c8ee24) |
✅ Actions performedUnit Test Generation started... |
|
@coderabbitai generate unit tests |
|
✅ Unit tests generated successfully! Generated 5 test files. The changes have been created in this pull request: View PR |
✅ Actions performedUnit Test Generation started... Unit tests generated successfully. |
1 participant
Gör hela cellen klickbar för interagerbara element.