feat(select-wrapper,base-select,grid-select): mobile-first base-select and grid-select development #3871
Conversation
WalkthroughThis PR introduces a new BaseSelect renderless component across Aurora and SaaS design packages, refactors grid-select state management and UI integration, adds comprehensive styling and animation variables, and updates mobile-first BaseSelect with new props, events, and directives to support enhanced tag and panel rendering modes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BaseSelect
participant Panel
participant Grid
participant Renderless
User->>BaseSelect: Interact (select/search/expand)
activate BaseSelect
BaseSelect->>Renderless: Emit event (select-change/radio-change)
activate Renderless
Renderless->>Renderless: Update state.modelValue<br/>Update state.currentKey
Renderless-->>BaseSelect: Return updated state
deactivate Renderless
alt Panel Slot Present
BaseSelect->>BaseSelect: Clear placeholder
BaseSelect->>Panel: Render with grid data
activate Panel
Panel->>Grid: Display rows with selection
User->>Grid: Select row
deactivate Panel
else No Panel Slot
BaseSelect->>BaseSelect: Show/hide tags<br/>based on state
end
BaseSelect->>BaseSelect: Apply tag rendering mode<br/>(showAllTextTag/hoverExpand)
BaseSelect-->>User: Update UI
deactivate BaseSelect
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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
🧹 Nitpick comments (7)
packages/design/aurora/src/base-select/index.ts (1)
47-98: DuplicatedtoggleCheckAlllogic between aurora and saas themes.The
toggleCheckAllfunction implementation is nearly identical in bothpackages/design/aurora/src/base-select/index.tsandpackages/design/saas/src/base-select/index.ts. Consider extracting this shared logic to the core renderless module (packages/renderless/src/base-select/index.ts) to avoid duplication and ensure consistent behavior across themes.Additionally, the same potential issue exists here where
valuecan remain undefined ifstate.selectClsdoesn't match any expected case (lines 70-79).packages/vue/src/grid-select/src/mobile-first.vue (1)
43-44: Complex data binding may benefit from extraction to computed property.The inline data binding expression on line 44 handles multiple data formats. While functional, this complexity in the template could be moved to a computed property in the renderless layer for better maintainability and testability.
:data="Array.isArray(state.gridData) ? state.gridData : state.gridData?.data || state.gridData || []"packages/vue/src/base-select/src/mobile-first.vue (5)
60-61: Clarify overlapping overflow classes for hoverExpandWhen
hoverExpand && (state.inputHovering || state.visible)is true, the element can receive bothoverflow-y-hidden(from Line 57) andoverflow-y-auto(Line 60). Tailwind will resolve this, but the intent is less clear.Consider simplifying to a single conditional that decides the vertical overflow mode to avoid conflicting utilities, e.g. by folding Line 57’s condition into the new one.
129-199:showAllTextTagand collapse-tag behaviors—double‑check UX detailsThe new
showAllTextTagbranch and the hover‑expand collapse tag wiring look reasonable overall, but there are a couple of edge behaviors worth checking:
- The collapse tag for hoverExpand (
data-tag="tags-collapse") is shown wheneverhoverExpandis true, regardless of whether anything is actually collapsed. This can lead to showing+ 0. You may want to gate it onstate.collapseTagsLength > 0.- The “all text” tag path bypasses the label slot (shows
allText || t('ui.base.all')directly). If consumers expect to customize this via thelabelslot, consider documenting this difference or offering a dedicated slot.Example tweak for the collapse tag:
- <tiny-tag - v-if="hoverExpand" + <tiny-tag + v-if="hoverExpand && state.collapseTagsLength > 0" :class=" m( gcls('tag-info'), { 'visible static': hoverExpand }, { 'invisible absolute': hoverExpand && (state.inputHovering || state.visible || state.isHidden) } ) "
369-379: Panel/header/footer slots and default panel coexistenceThe new
header,panel, andfooterslots significantly improve extensibility. A couple of points to verify:
- The
panelslot is additive: it renders before theoptimization/tiny-scrollbarblocks, so custom panel content will appear above the default option list, not replace it. If the design is that apanelslot fully owns the panel (e.g., for grid-select), you may want to wrap the default list inv-if="!slots.panel"as well to avoid double content.- The empty-state template is already guarded with
!slots.panel, which is good—it prevents default “no data” content from fighting with a custom panel.If
panelis intended to replace the core list when supplied, adjusting the list block like this would make that explicit:- <template v-if="optimization"> + <template v-if="optimization && !slots.panel"> @@ - <tiny-scrollbar - v-if="!optimization" + <tiny-scrollbar + v-if="!optimization && !slots.panel"Also applies to: 523-525, 539-540
561-569: Renderless switch, new directives/components/icons, and emitsScript-side wiring is coherent:
- Importing from
@opentiny/vue-renderless/base-select/vueinstead of the old select path matches the PR intent to use the new base-select renderless logic.TinyButton, additional icons, andAutoTipare registered and ready for use by new slots/panels.- Adding
'confirm'and'top-create-click'toemitskeeps Vue 3 event declarations in sync with what the renderless layer likely emits.Given these are public-surface changes, make sure:
- Corresponding documentation is updated (props/events/slots and examples).
- Any existing usages that relied on the old renderless entry or icon assumptions are updated or covered by tests.
I can help sketch doc/test updates for the new events and slot APIs if you share the expected usage patterns.
Also applies to: 577-584, 615-617, 620-621, 635-652
726-732: New props (allText,maxTagWidth,tagType,clearNoMatchValue,showAllTextTag,hoverExpand)The additional props are neatly grouped at the end and map directly to new behaviors in the template/state:
allTextandshowAllTextTagdrive the “select all” label and tag rendering.maxTagWidthis fed into multipleTinyTaginstances for consistent truncation.tagTypeandclearNoMatchValue/hoverExpandare consumed via renderless state and template classes.From an API perspective, this is a non-trivial expansion; consider:
- Documenting default values and interactions (e.g.,
showAllTextTagvsshowAlloption, and whenhoverExpandshould be used).- Adding targeted tests around multi-select tag rendering and “all” behaviors to catch regressions.
If you’d like, I can propose a small matrix of test cases for
showAllTextTag×hoverExpand×multipleto validate the new paths.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/design/saas/src/base-select/icon-loading.svgis excluded by!**/*.svg
📒 Files selected for processing (17)
packages/design/aurora/index.ts(2 hunks)packages/design/aurora/src/base-select/index.ts(1 hunks)packages/design/saas/index.ts(2 hunks)packages/design/saas/src/base-select/index.ts(1 hunks)packages/renderless/src/base-select/index.ts(9 hunks)packages/renderless/src/grid-select/index.ts(2 hunks)packages/renderless/src/grid-select/vue.ts(1 hunks)packages/theme-saas/src/base-select/index.less(1 hunks)packages/theme/build/mapVar.js(1 hunks)packages/theme/src/index.less(1 hunks)packages/vue/mobile-first.ts(3 hunks)packages/vue/src/base-select/src/mobile-first.vue(19 hunks)packages/vue/src/grid-select/index.ts(1 hunks)packages/vue/src/grid-select/src/index.ts(1 hunks)packages/vue/src/grid-select/src/mobile-first.vue(1 hunks)packages/vue/src/grid-select/src/pc.vue(3 hunks)packages/vue/src/tag/src/token.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2024-11-25T03:43:05.285Z
Learnt from: Davont
Repo: opentiny/tiny-vue PR: 2513
File: packages/vue/src/huicharts/huicharts-histogram/src/chart-histogram.vue:33-36
Timestamp: 2024-11-25T03:43:05.285Z
Learning: 在 Tiny Vue 代码库中，使用 `chart-core` 中的 `huiChartOption` 的组件，不应在其 `data` 中定义 `huiChartOption` 或 `option`，而是应该依赖 `chart-core` 提供的 `huiChartOption`。
Applied to files:
packages/design/saas/src/base-select/index.tspackages/design/aurora/src/base-select/index.tspackages/vue/src/base-select/src/mobile-first.vue
📚 Learning: 2024-11-25T03:43:19.322Z
Learnt from: Davont
Repo: opentiny/tiny-vue PR: 2513
File: packages/vue/src/huicharts/huicharts-heatmap/src/chart-heatmap.vue:38-40
Timestamp: 2024-11-25T03:43:19.322Z
Learning: 在 Vue.js 组件（如 `chart-heatmap.vue`）中，使用来自 `chart-core` 的 `huiChartOption` 来管理图表选项，不要在 `data()` 中声明 `option`。
Applied to files:
packages/design/saas/src/base-select/index.tspackages/design/aurora/src/base-select/index.ts
📚 Learning: 2024-11-25T03:24:05.740Z
Learnt from: Davont
Repo: opentiny/tiny-vue PR: 2513
File: packages/vue/src/huicharts/huicharts-sunburst/src/chart-sunburst.vue:30-32
Timestamp: 2024-11-25T03:24:05.740Z
Learning: 在位于`packages/vue/src/huicharts/huicharts-sunburst/src/chart-sunburst.vue`的组件中，当使用`chart-core`时，应删除错误的`option`定义，使用`chart-core`中的`huiChartOption`。
Applied to files:
packages/design/saas/src/base-select/index.tspackages/design/aurora/src/base-select/index.ts
📚 Learning: 2024-11-25T03:24:33.808Z
Learnt from: Davont
Repo: opentiny/tiny-vue PR: 2513
File: packages/vue/src/huicharts/huicharts-sunburst/src/chart-sunburst.vue:30-32
Timestamp: 2024-11-25T03:24:33.808Z
Learning: 对于使用了来自`opentiny/vue-huicharts-core`中`Core` mixin的组件，`huiChartOption`数据属性已由`chart-core`提供，因此无需在组件的`data()`中声明。
Applied to files:
packages/design/saas/src/base-select/index.ts
📚 Learning: 2024-11-04T09:35:13.159Z
Learnt from: zzcr
Repo: opentiny/tiny-vue PR: 2481
File: packages/theme/src/time-range/vars.less:27-28
Timestamp: 2024-11-04T09:35:13.159Z
Learning: 在 `packages/theme/src/time-range/vars.less` 文件中，应使用 `var(--tv-TimeRange-header-height)` 作为 `--tv-TimeRange-header-line-height` 的值，以保持一致性。
Applied to files:
packages/theme/build/mapVar.js
🧬 Code graph analysis (2)
packages/vue/src/grid-select/index.ts (1)
packages/vue/mobile-first.ts (2)
GridSelect(303-303)GridSelect(304-304)
packages/vue/src/grid-select/src/index.ts (1)
packages/vue-common/src/index.ts (4)
$prefix(43-43)$props(45-53)props(55-73)$setup(199-233)
⏰ 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: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (29)
packages/vue/src/tag/src/token.ts (1)
48-48: Confirm intent:fill-light-warningno longer uses the-subtletokenAll other
fill-light-*entries use the*-subtlefill as the base color and a stronger color on hover. Switchingfill-light-warningtofill-color-warningbreaks that pattern and will make warning icons visually stronger than the other light variants. Please confirm this is aligned with the latest design tokens/spec; otherwise consider reverting tofill-color-warning-subtlefor consistency.packages/theme/src/index.less (1)
90-90: Inconsistency between AI summary and provided code.The AI-generated summary claims two identical imports of
'./motion/index.less'were added (one after month-range and one after month-table), but the provided code shows only one import at line 90 (positioned after month-table and before nav-menu). The import location and alphabetical ordering at line 90 are correct, but the discrepancy with the summary should be clarified.Can you confirm whether there is indeed a second import of
'./motion/index.less'elsewhere in this file, or if the AI summary incorrectly reported duplicate additions?packages/theme/build/mapVar.js (1)
523-619: Animation variables addition needs verification for actual usage.The animation variable definitions are syntactically correct and follow consistent naming conventions with the
--tv-motion-*pattern. However, the review requires verification of:
- Whether these variables are actually consumed in the component files mentioned in the PR context (base-select, grid-select)
- Whether corresponding LESS/CSS files reference these animation properties
- Any potential duplication of this section within mapVar.js or related files
Please provide confirmation that these motion variables are integrated into their corresponding animation modules (ants.less, arrow.less, bounce.less, etc.).
packages/renderless/src/grid-select/index.ts (2)
59-60: LGTM! State synchronization improvements.The changes ensure
state.currentKeyandstate.modelValuestay properly synchronized with the selected row in single-select mode, which is essential for consistent v-model integration and radio configuration.Also applies to: 64-64, 540-541
127-127: Explicit configuration improves clarity.The
immediate: falseis already the default behavior in Vue watchers, so this change doesn't alter functionality but makes the intent explicit and improves code readability.packages/design/aurora/index.ts (1)
30-30: LGTM! Consistent component registration.The BaseSelect component is properly imported and exported following the existing pattern used by other components in the Aurora design package.
Also applies to: 65-66
packages/design/saas/index.ts (1)
33-33: LGTM! Consistent component registration.The BaseSelect component is properly imported and exported following the established pattern, maintaining consistency with the Aurora design package implementation.
Also applies to: 71-72
packages/theme-saas/src/base-select/index.less (1)
1-415: LGTM! Comprehensive base-select styling.The styling implementation is well-structured with:
- Proper responsive design with mobile breakpoints
- Browser compatibility for IE10-11 and Edge
- CSS variables for theming flexibility
- Clear organization and comments
The use of Tailwind utilities and modern CSS features aligns with the component's mobile-first approach.
packages/vue/src/grid-select/src/pc.vue (2)
28-32: LGTM! Remote search support properly integrated.The remote search props (
remote,remoteMethod,initQuery,extraQueryParams,remoteConfig) are correctly passed through to the base-select component, enabling remote data fetching for grid-select.
56-56: Verify prop spreading doesn't cause conflicts.The spread
...propsat line 68 imports shared props from@opentiny/vue-common, but explicit prop definitions follow. Ensure the spread doesn't override any explicitly defined props or introduce unexpected behavior. Unable to verify the specific props included in the spread without codebase access—manually confirm that no property keys in the importedpropsobject conflict with explicitly defined component props.packages/renderless/src/base-select/index.ts (3)
280-283: LGTM! Ensures currentLabel consistency.Propagating
currentLabelfrom the option tooption.state.currentLabelwhen missing ensures the label is available in the expected location for downstream consumers.
293-293: LGTM! More robust label checking.The updated conditions check multiple possible locations for labels (
option.label,option.currentLabel,option.state.currentLabel), preventing inadvertent clearing of valid but unlabeled options.Also applies to: 309-310
331-333: LGTM! Panel-slot aware placeholder handling.The additions properly handle placeholder visibility when using custom panel slots. When
vm.$slots.panelexists and items are selected, the placeholder is correctly cleared to avoid visual conflicts. Thevmparameter propagation enables this behavior consistently across visibility transitions.Also applies to: 347-350, 1139-1142, 1217-1217, 1240-1246, 1281-1281
packages/vue/src/grid-select/index.ts (1)
12-12: LGTM! Enhanced v-model integration.The import path change to
'./src/index'supports multi-platform implementations, and theGridSelect.modelproperty properly exposes v-model configuration for consumers. This is a beneficial public API enhancement.packages/design/saas/src/base-select/index.ts (3)
1-11: LGTM!The icon imports and base configuration are properly set up. The saas theme appropriately includes additional icons (
addIcon,loadingIcon) compared to the aurora theme, which aligns with the design differences mentioned in the comments.
67-77: Potential undefinedvalueifstate.selectClsis not one of the expected values.In the non-filtered branch (lines 68-76), if
state.selectClsis not'check','halfselect', or'checked-sur', thevaluevariable remainsundefined. This could cause issues whenapi.updateModelValue(value)is called on line 93.Consider adding a fallback or early return:
} else if (state.selectCls === 'checked-sur') { value = [] + } else { + return // No action needed for unknown selectCls } }
96-104: LGTM!The
computedShowTagTextandisTagClosablehelper functions are straightforward and well-documented with comments explaining the behavioral differences from the default theme.packages/vue/src/grid-select/src/index.ts (2)
16-28: LGTM!The component definition follows TinyVue conventions correctly. The use of
modelValue: {}allows flexibility for both single (any type) and multiple (array) selection modes.
79-117: LGTM!The grid and selection configuration props have sensible defaults. The
checkMethodreturningtrueby default allows all rows to be selectable, which is the expected behavior for an unrestricted grid-select.packages/design/aurora/src/base-select/index.ts (2)
42-45: LGTM - Aurora-specific dropdown icon visibility logic.The
computedShowDropdownIconfunction correctly handles the visibility of the dropdown icon when remote search with filterable is enabled.
99-107: LGTM!The
computedShowTagTextandisTagClosablehelpers are consistent with the saas implementation and properly documented.packages/vue/src/grid-select/src/mobile-first.vue (3)
55-66: LGTM!The component setup correctly imports the renderless API and registers child components with the
Tinyprefix convention.
167-169: LGTM!The setup function correctly delegates to the renderless layer, following the TinyVue composition pattern.
67-166: Ensure props stay synchronized between index.ts and mobile-first.vue.The props definition in this file should mirror
packages/vue/src/grid-select/src/index.tsfor the virtual-template pattern to work correctly. If each template can be loaded independently, verify that all prop definitions remain synchronized between the two files. Consider adding a comment in the code documenting this dependency or implementing a synchronization check in the build process.packages/vue/src/base-select/src/mobile-first.vue (4)
4-4: Rootdata-tagaddition looks consistentAdding
data-tag="tiny-base-select"on the root wrapper matches existing Tiny components’ tagging patterns and should help downstream directives/tools; no functional issues here.
86-91: Use ofstate.getTagTypeandmaxTagWidthon tagsWiring
:type="state.getTagType"and:maxWidth="maxTagWidth"into the main and “+N” tags is consistent and keeps appearance driven from state/props. AssumingTinyTagexposes amaxWidthprop, this is a clean way to control truncation without layout hacks.Also applies to: 121-123
263-263:tiny_mode="mobile-first"and caret icon viastate.getIcon.icon
- Adding
tiny_mode="mobile-first"ontiny-inputaligns this with other mobile-first components and keeps styling centralized.- Swapping the fixed caret icon for
<component :is="state.getIcon.icon">is a good move toward state-driven icon selection, and matches the comment that this path should be used.Just make sure that
state.getIcon.iconalways resolves to a registered component (e.g.,'IconDownWard','IconSearch', etc.) and never tonull/undefined, so the caret doesn’t silently disappear in some states.To confirm, you can search the renderless
base-selectAPI for all possible values ofgetIcon.iconand ensure they correspond to components registered here.Also applies to: 313-317
424-424: Scroll/list conditions and “select all” option styling
- Using
v-if="!optimization"ontiny-scrollbarin addition to theoptimizationtemplate is redundant but harmless; it does clearly document the mutual exclusivity.- The updated class strings for the “select all” rows (both unfiltered and filtered) look consistent with the rest of the dropdown (height, padding, typography) and the icon classes unify to Tailwind-style utilities.
These changes are mostly cosmetic and help keep appearance in sync between filtered and unfiltered “all” states.
Also applies to: 438-457, 474-493
packages/renderless/src/grid-select/vue.ts (1)
120-128: Explicitimmediate: falseonextraQueryParamswatch is reasonableMaking the non-immediate behavior explicit while keeping
deep: truealigns with the existing logic (only refilter on actual param changes, not on mount). No issues from the renderless side.
| import Grid from '@opentiny/vue-grid' | ||
| import GridSelect from '@opentiny/vue-grid-select' |
There was a problem hiding this comment.
Grid and GridSelect missing from default export object.
The named exports (lines 301-304) include Grid, TinyGrid, GridSelect, and TinyGridSelect, but the default export object starting at line 463 does not include these entries. All other components follow the pattern of being exported in both places.
Add Grid and GridSelect to the default export object (after ColumnListItem entries around line 519):
ColumnListItem,
TinyColumnListItem: ColumnListItem,
+ Grid,
+ TinyGrid: Grid,
+ GridSelect,
+ TinyGridSelect: GridSelect,
Currency,
TinyCurrency: Currency,Also applies to: 140-141, 301-304
🤖 Prompt for AI Agents
packages/vue/mobile-first.ts lines 29-30 (and referencing named exports at
301-304 and default export around 463/519): the default export object is missing
Grid and GridSelect even though they are present as named exports; add entries
for Grid and GridSelect into the default export object immediately after the
ColumnListItem entries (around line 519) so the pattern matches other
components; also check the other spots noted (lines 140-141 and 301-304) to
ensure the named exports are correct and consistent with the default export.
2 participants
PR
feat:多端基础下拉以及下拉表格开发
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.