tiny-vue icon indicating copy to clipboard operation
tiny-vue copied to clipboard

[WIP]fix(dialog-select): dialog-select set-selection render checkbox error

Open gausszhou opened this issue 3 months ago β€’ 1 comments

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • [ ] The commit message follows our Commit Message Guidelines
  • [x] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • [ ] Bugfix
  • [ ] Feature
  • [ ] Code style update (formatting, local variables)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [x] CI related changes
  • [ ] Documentation content changes
  • [ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • [ ] Yes
  • [x] No

Other information

Summary by CodeRabbit

  • Tests
    • Expanded selection test to expect the first two rows selected after interaction, improving coverage for multi-row selection.
  • Chores
    • Added grid-related type definitions (events, columns, config) to improve typings and maintainability.
  • Refactor
    • Rewrote a prop membership check to use a clearer includes-based validator without changing accepted values.

gausszhou avatar Sep 13 '25 13:09 gausszhou

Walkthrough

Adds new grid TypeScript interfaces and a Partial type, tightens a DialogSelect prop validator to use Array.includes, and updates a dialog-select test to expect two selected rows (indices 0 and 1) instead of one.

Changes

Cohort / File(s) Summary
Dialog select test
examples/sites/demos/pc/app/dialog-select/set-selection.spec.ts
Broadened selection assertion by changing loop condition from i === 1 to i <= 1, so rows 0 and 1 are expected to have row__selected.
Grid types added
packages/renderless/types/grid.type.ts
Added interfaces IGridEvent { selectAll: Function }, IGridColumn { field: string; title: string }, IGridConfig { columns: IGridColumn[]; data: any[]; border: boolean; event: IGridEvent; treeConfig: any; selectConfig: any } and IGriOp = Partial<IGridConfig>.
DialogSelect prop validator
packages/vue/src/dialog-select/src/index.ts
Replaced validator ~['grid','tree'].indexOf(value) with typed validator: (value: string) => ['grid','tree'].includes(value).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Tester as Test Runner
  participant Dialog as DialogSelect Component
  participant Grid as Grid Renderer

  Tester->>Dialog: open dialog, trigger selection actions
  Dialog->>Grid: toggle/select rows (index 1, then others)
  Grid-->>Dialog: emit updated selection state
  Dialog-->>Tester: render rows with `row__selected` classes

  rect rgba(200,230,255,0.28)
    note right of Dialog: Test expectation changed to include index 0 and 1 (i <= 1)
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • opentiny/tiny-vue#3702 β€” Related: test change broadens selection expectation and may address reported DialogSelect behavior where the first option wasn't selected on initial open.

Poem

I nibbled through the lines at night, two rows now sparkle in my sight,
Props are strict and types align, selections shine in tidy kind.
A rabbit hops β€” the tests all run β€” two little rows, a job well done. πŸ‡βœ¨

✨ Finishing touches
  • [ ] πŸ“ Generate Docstrings
πŸ§ͺ Generate unit tests
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

πŸ“œ Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between d22e667584c138068cbbaa15eac7a465bf8af971 and 6ffd0045931bf3c273c7200bda674cbf16b52299.

πŸ“’ Files selected for processing (3)
  • examples/sites/demos/pc/app/dialog-select/set-selection.spec.ts (1 hunks)
  • packages/renderless/types/grid.type.ts (1 hunks)
  • packages/vue/src/dialog-select/src/index.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • examples/sites/demos/pc/app/dialog-select/set-selection.spec.ts
  • packages/vue/src/dialog-select/src/index.ts
  • packages/renderless/types/grid.type.ts

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.

❀️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Pre-merge checks

βœ… Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage βœ… Passed No functions found in the changes. Docstring coverage check skipped.
Title Check βœ… Passed The title clearly identifies the primary change β€” a fix in dialog-select's set-selection checkbox rendering β€” which aligns with the test and source edits in the PR and communicates the main developer intent. It is concise and focused on the bug being addressed. The inclusion of a "[WIP]" prefix and the duplicated "dialog-select" are minor noise but do not make the title misleading.
Description Check βœ… Passed Check skipped - CodeRabbit’s high-level summary is enabled.

coderabbitai[bot] avatar Sep 13 '25 13:09 coderabbitai[bot]