feat: order #3677
feat: order #3677
Conversation
WalkthroughAdds a new Space component across the codebase: renderless logic, Vue PC/mobile-first implementations, package entry, tests, and documentation. Introduces demos (PC and mobile-first), API metadata, menus, and module registry updates. Implements child ordering, gap computation, and layout controls via props (size, align, justify, direction, wrap, order). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant TinySpace as TinySpace (PC/MF)
participant Renderless as renderless (state/orderedChildren)
participant DOM as DOM
App->>TinySpace: Render with props (size, direction, align, justify, wrap, order)
TinySpace->>Renderless: setup(props, slots)
Renderless-->>TinySpace: { state.gapStyle, orderedChildren }
TinySpace->>DOM: div[data-tag="tiny-space"] with styles (gap, flex, wrap)
TinySpace->>DOM: Render ordered children
note over TinySpace,Renderless: If order provided, children sorted by keys
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (40)
examples/sites/demos/mobile-first/app/space/webdoc/space.cn.md (1)
1-9: Polish: add meta description and avoid unnecessary HTML wrapperSuggest adding a description for SEO and switching the body copy to plain Markdown for consistency with other docs.
--- title: Space 间距 +description: 控制界面元素之间间距的布局组件，用于提升整体布局的清晰度与美观性。 --- -# Space 间距 - -<div> -主要用于控制界面元素之间的空白区域，帮助提升整体布局的清晰度和美观性。 -</div> +# Space 间距 + +主要用于控制界面元素之间的空白区域，帮助提升整体布局的清晰度和美观性。examples/sites/demos/pc/app/space/webdoc/space.cn.md (1)
1-9: Polish: add meta description and drop the HTML wrapperMirror the CN mobile-first page improvements here for consistency.
--- title: Space 间距 +description: 控制界面元素之间间距的布局组件，用于提升整体布局的清晰度与美观性。 --- -# Space 间距 - -<div> -主要用于控制界面元素之间的空白区域，帮助提升整体布局的清晰度和美观性。 -</div> +# Space 间距 + +主要用于控制界面元素之间的空白区域，帮助提升整体布局的清晰度和美观性。examples/sites/demos/pc/app/space/webdoc/space.en.md (1)
1-9: Fix EN title/heading to avoid mixed-language UI; refine copyUse English-only title/heading on the EN page and tighten the description.
--- -title: Space 间距 +title: Space +description: A layout utility for controlling spacing between elements to improve overall clarity and aesthetics. --- -# Space 间距 +# Space -<div> -It is mainly used to control the blank area between interface elements to help improve the clarity and aesthetics of the overall layout. -</div> +Use Space to control the spacing between interface elements and improve overall layout clarity and aesthetics.examples/sites/demos/mobile-first/app/space/webdoc/space.en.md (1)
1-9: Align EN page with English-only title/heading; refine descriptionSame change as PC EN to keep the docs consistent.
--- -title: Space 间距 +title: Space +description: A layout utility for controlling spacing between elements to improve overall clarity and aesthetics. --- -# Space 间距 +# Space -<div> -It is mainly used to control the blank area between interface elements to help improve the clarity and aesthetics of the overall layout. -</div> +Use Space to control the spacing between interface elements and improve overall layout clarity and aesthetics.examples/sites/demos/mobile-first/menus.js (1)
129-130: MoveSpaceinto the Container group and remove trailing comma in Others
Space is a layout/container utility—place it under “容器组件/Container” for IA parity and discoverability. After relocating it, remove the trailing comma from the last item in “其它/Others.”Apply this diff in
examples/sites/demos/mobile-first/menus.js:- { name: 'Divider', nameCn: '分割线', key: 'divider' }, - { name: 'Space', nameCn: '间距', key: 'space' } + { name: 'Divider', nameCn: '分割线', key: 'divider' }Then add to the Container group’s children:
{ name: 'Space', nameCn: '间距', key: 'space' }examples/sites/demos/pc/menus.js (1)
107-108: Space menu entry verified — demos & registry presentAll PC Space demos and webdoc entries exist and match the
spacemenu key; route wiring resolves correctly. Consider adding ameta.stabletag (similar to VirtualScrollBox) if Space is stable.examples/sites/demos/mobile-first/app/space/space-wrap.vue (1)
3-3: Correct the copy-pasted commentThis demo toggles wrapping, not row/column selection.
- <!-- 选择行或列的按钮 --> + <!-- 选择是否换行的按钮 -->examples/sites/demos/mobile-first/app/space/space-size.vue (3)
3-3: Align the comment with the demo purposeThis demo adjusts spacing, not row/column selection.
- <!-- 选择行或列的按钮 --> + <!-- 设置行/列间距 -->
8-14: Avoid clickable “labels” to reduce UX confusionThese buttons look actionable but serve as labels for sliders. Consider disabling them.
- <tiny-button type="primary">行</tiny-button> + <tiny-button type="primary" disabled>行</tiny-button> ... - <tiny-button type="success">列</tiny-button> + <tiny-button type="success" disabled>列</tiny-button>
16-16: Fix comment: it’s size, not directionMinor doc consistency.
- <!-- 使用 dynamic direction 值 --> + <!-- 使用动态 size 值 -->examples/sites/demos/mobile-first/app/space/space-align.vue (3)
4-4: Bind numeric size instead of string.
size="4"is a string; the prop accepts numbers too. Use:size="4"to avoid string/number ambiguity in downstream logic.- <tiny-space direction="row" size="4" style="margin-bottom: 10px"> + <tiny-space direction="row" :size="4" style="margin-bottom: 10px">
5-7: Indicate active selection for better UX.Highlight the currently selected align option.
- <tiny-button style="margin: 0" v-for="val in alignOptions" :key="val" @click="setAlign(val)"> + <tiny-button + style="margin: 0" + v-for="val in alignOptions" + :key="val" + :type="val === alignValue ? 'primary' : 'default'" + @click="setAlign(val)" + >
16-36: Unify TS usage and improve type inference.This file uses TS with Options API but without
defineComponent, while other demos mix JS/TS. ConsiderdefineComponentor<script setup lang="ts">for consistency and better typing.-<script lang="ts"> -import { TinyButton, TinySpace } from '@opentiny/vue' - -export default { +<script lang="ts"> +import { defineComponent } from 'vue' +import { TinyButton, TinySpace } from '@opentiny/vue' + +export default defineComponent({ components: { TinyButton, TinySpace }, data() { const alignOptions = ['start', 'center', 'end', 'baseline', 'stretch'] return { alignValue: 'start', alignOptions } }, methods: { setAlign(val: string) { this.alignValue = val } } -} +}) </script>examples/sites/demos/mobile-first/app/space/basic-usage.vue (2)
8-8: Make slider width responsive for mobile-first.Avoid fixed 300px on small screens.
- <tiny-slider v-model="value" :min="0" :max="50" :step="2" style="width: 300px; margin-bottom: 20px" /> + <tiny-slider + v-model="value" + :min="0" + :max="50" + :step="2" + style="width: 100%; max-width: 300px; margin-bottom: 20px" + />
16-37: Align script style across demos.Other Space demos use TS; this one is JS. Consider switching to TS or
<script setup>to keep the suite consistent.examples/sites/demos/mobile-first/app/space/space-justify.vue (3)
4-4: Bind numeric size instead of string.- <tiny-space direction="row" size="4" style="margin-bottom: 10px"> + <tiny-space direction="row" :size="4" style="margin-bottom: 10px">
5-7: Show selected justify option.- <tiny-button v-for="val in justifyOptions" :key="val" @click="setJustify(val)"> + <tiny-button + v-for="val in justifyOptions" + :key="val" + :type="val === justifyValue ? 'primary' : 'default'" + @click="setJustify(val)" + >
18-38: Consistent TS pattern.Same note as align demo: wrap with
defineComponentor use<script setup lang="ts">for stronger typing.packages/vue/src/space/src/token.ts (1)
1-34: Narrow the token map type and export keys.Add
as constand a key type to improve developer experience and prevent accidental key typos.-export const classes = { +export const classes = { 'base': 'flex', @@ 'wrap-true': 'flex-wrap', 'wrap-false': 'flex-nowrap' -} +} as const + +export type SpaceClassKey = keyof typeof classespackages/renderless/types/space.type.ts (3)
20-20: Tighten theordertype.
any[]is too loose. Suggest a minimal, forward-compatible union.- order?: any[] // 可以拓展为具体对象结构 + // 支持简单序号或键值+序号对象；后续可扩展更复杂结构 + order?: Array<number | string | { key: number | string; order: number }>
22-22: RefinecustomStyletyping.Allow numeric values; avoid
any.- customStyle?: string | Record<string, any> + customStyle?: string | Record<string, string | number>
32-32: BroadengapStylevalue type.Inline styles often use numbers.
- gapStyle: Record<string, string> // 一般是 margin 样式，如 { gap: '12px' } + gapStyle: Record<string, string | number> // 如 { gap: '12px' } 或 { gap: 12 }examples/sites/demos/apis/space.js (2)
10-11: Docs type string: align with house style.Use "string | number | array" (or the schema’s canonical form) instead of "[string, number, array]" for consistency with other API docs.
- type: '[string, number, array]', + type: 'string | number | array',
13-15: Clarify array order vs CSS gap shorthand to avoid confusion.Docs say size as [horizontal, vertical], while CSS gap is "row-gap column-gap" = [vertical, horizontal]. Add a note to make this explicit.
'en-US': 'Set the spacing size. Can be a string, number, or an array like [horizontal, vertical]' + // Note: gap shorthand is 'row-gap column-gap' => [vertical, horizontal] when applied.Also applies to: 20-31
packages/renderless/types/index.ts (1)
166-168: Keep barrel exports alphabetically ordered.Place space.type before split.type for consistency with the surrounding ordering.
-export * from './split.type' -export * from './space.type' +export * from './space.type' +export * from './split.type'examples/sites/demos/pc/app/space/space-order.vue (1)
2-7: Use vnode.key-based ordering in tiny-space
tiny-space reorders children by their vnode.key; add a key to the fourth button and note in the demo thatorderrefers to vnode keys.- <tiny-button>Fourth Button</tiny-button> + <tiny-button key="4">Fourth Button</tiny-button>examples/sites/demos/pc/app/space/space-wrap.vue (1)
12-31: Nit: Optional TS typing via defineComponent.
If you want TS inference on data/methods, wrap the options in defineComponent; current code runs fine without it.Apply if desired:
-<script lang="ts"> -import { TinyButton, TinySpace } from '@opentiny/vue' +<script lang="ts"> +import { defineComponent } from 'vue' +import { TinyButton, TinySpace } from '@opentiny/vue' -export default { +export default defineComponent({ components: { TinySpace, TinyButton }, data() { return { wrapValue: false } }, methods: { setWrap(val: boolean) { this.wrapValue = val } } -} +}) </script>packages/vue/src/space/__tests__/space.test.tsx (1)
61-75: Optional: broaden slot checks and add justify/order coverage.
Add a test for justify, and one asserting an item’s CSS order when prop/order is set.Example additions (sketch):
+ test('props justify', async () => { + const wrapper = mount(() => <Space justify="space-between"><span>a</span><span>b</span></Space>) + const container = wrapper.find('[data-tag="tiny-space"]') + expect(container.attributes('style')).toContain('justify-content: space-between') + wrapper.unmount() + }) + + test('props order', async () => { + const wrapper = mount(() => ( + <Space order={[2,1]}> + <span class="a">A</span> + <span class="b">B</span> + </Space> + )) + // Adjust selector/expectation to match implementation details + const items = wrapper.findAll('[data-tag="tiny-space"] > *') + expect(items[0].attributes('style') + items[1].attributes('style')).toMatch(/order:/) + wrapper.unmount() + })examples/sites/demos/pc/app/space/space-direction.vue (1)
27-29: Nit: guard input values in the demo method.
If you want to keep examples robust, constrain to 'row' | 'column'.- setDirection(direction) { + setDirection(direction) { this.direction = direction }Or (optional TS in demo):
-<script> +<script lang="ts"> +type Dir = 'row' | 'column' ... - setDirection(direction) { + setDirection(direction: Dir) {packages/vue/src/space/index.ts (1)
17-19: Nit: align naming with Vue 3 app API (param name).
Renaming the param to app improves clarity without behavior change.-Space.install = function (Vue) { - Vue.component(Space.name, Space) +Space.install = function (app) { + app.component(Space.name, Space) }examples/sites/demos/mobile-first/app/space/webdoc/space.js (2)
24-27: Document array order for size to prevent ambiguity.Spell out that size as an array is [horizontal, vertical] (maps to CSS gap: row column). This avoids confusion with row/column UI labels.
- 'zh-CN': '<p>通过 `size` 属性设置间距，支持 small / medium / large 或自定义数值 / 数组。</p>', + 'zh-CN': '<p>通过 `size` 属性设置间距，支持 small / medium / large 或自定义数值 / 数组（[horizontal, vertical]）。</p>', - 'en-US': - '<p>Use the `size` prop to define spacing. Supports small / medium / large or custom values / arrays.</p>' + 'en-US': + '<p>Use the `size` prop to define spacing. Supports small / medium / large or custom values / arrays as [horizontal, vertical].</p>'
87-89: Call out the preferred identifier for order (recommend setting :key).Since implementation may fall back to class names, suggest users set a stable :key on children for predictable ordering.
packages/renderless/src/space/vue.ts (1)
22-27: More robust VNode detection and comment filtering (optional).Use Vue’s isVNode and Comment symbol for accuracy, and skip Text nodes too if they shouldn’t render space slots.
Outside the selected range, add imports:
import { isVNode, Comment, Text } from 'vue'Then adjust:
-const validChildren = children.filter((v) => { - if (!isVNodeFn(v)) return false - const type = (v as any).type - return type !== 'Comment' && type !== Symbol.for('v-comment') -}) +const validChildren = children.filter((v) => { + if (!isVNode(v)) return false + const type = (v as any).type + return type !== Comment && type !== Text +})Also applies to: 7-9
examples/sites/demos/pc/app/space/webdoc/space.js (1)
37-38: Minor CN copy nit: add space between “row” and “column”.Improves readability.
- 'zh-CN': '<p>通过 `direction` 属性设置排列方向，支持 row 或column。</p>', + 'zh-CN': '<p>通过 `direction` 属性设置排列方向，支持 row 或 column。</p>',packages/vue/src/space/src/pc.vue (1)
1-35: Remove large commented-out template block.Dead code adds noise and risks divergence.
-<!-- <template> - <div - data-tag="tiny-space" - :style="[ - state.gapStyle, - { - display: 'flex', - flexDirection: direction || 'row', - alignItems: align || 'flex-start', - justifyContent: justify || 'flex-start', - flexWrap: wrap ? 'wrap' : 'nowrap', - ...customStyle - } - ]" - :class="customClass" - > - <template v-for="(child, idx) in orderedChildren" :key="child.key ?? idx"> - <component :is="child" /> - </template> - </div> -</template> - -<script lang="ts"> -import { defineComponent, props, setup } from '@opentiny/vue-common' -import { renderless, api } from '@opentiny/vue-renderless/space/vue' -import type { ISpaceApi } from '@opentiny/vue-renderless/types/space.type' - -export default defineComponent({ - name: 'TinySpace', - props: [...props, 'size', 'direction', 'align', 'justify', 'wrap', 'order', 'customClass', 'customStyle'], - setup(props, context) { - return setup({ props, context, renderless, api }) as unknown as ISpaceApi - } -}) -</script> -->packages/vue/src/space/package.json (1)
5-5: Fill in package description.Empty
"description"reduces discoverability. Add a concise summary (e.g., “Flexible layout spacing container with ordering and gap controls for Tiny Vue”).packages/vue/src/space/src/index.ts (4)
7-11: Add a validator for tuple sizes to guard invalid arrays.Currently any array passes. Validate
[horizontal, vertical]shape.size: { type: [String, Number, Array] as PropType<string | number | [string | number, string | number]>, default: 'small' + ,validator: (v: unknown) => + typeof v === 'string' || + typeof v === 'number' || + (Array.isArray(v) && v.length === 2 && ['string','number'].includes(typeof v[0]) && ['string','number'].includes(typeof v[1])) },
23-26: Consider supporting 'space-evenly' in justify.Common flex distribution; add if not intentionally excluded.
- type: String as PropType<'start' | 'center' | 'end' | 'space-between' | 'space-around'>, + type: String as PropType<'start' | 'center' | 'end' | 'space-between' | 'space-around' | 'space-evenly'>,
42-45: Clarify and validate the semantics of order[].If
orderreferences child keys, consider validating uniqueness and providing dev-time warnings to avoid partial/duplicate ordering.order: { type: Array as PropType<string[]>, - default: () => [] + default: () => [], + validator: (arr: string[]) => Array.isArray(arr) && new Set(arr).size === arr.length },Optionally document expected values (slot keys, data-key, etc.).
62-64: Slots typing is fine; could be omitted for simplicity.
SlotsTypeis not strictly needed here; removing it can reduce TS friction across versions. Low priority.- slots: Object as SlotsType<{ - default: {} // 默认插槽 - }>, + // slots: infer from usage
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (36)
examples/sites/demos/apis/space.js(1 hunks)examples/sites/demos/mobile-first/app/space/basic-usage.vue(1 hunks)examples/sites/demos/mobile-first/app/space/space-align.vue(1 hunks)examples/sites/demos/mobile-first/app/space/space-direction.vue(1 hunks)examples/sites/demos/mobile-first/app/space/space-justify.vue(1 hunks)examples/sites/demos/mobile-first/app/space/space-order.vue(1 hunks)examples/sites/demos/mobile-first/app/space/space-size.vue(1 hunks)examples/sites/demos/mobile-first/app/space/space-wrap.vue(1 hunks)examples/sites/demos/mobile-first/app/space/webdoc/space.cn.md(1 hunks)examples/sites/demos/mobile-first/app/space/webdoc/space.en.md(1 hunks)examples/sites/demos/mobile-first/app/space/webdoc/space.js(1 hunks)examples/sites/demos/mobile-first/menus.js(1 hunks)examples/sites/demos/pc/app/space/basic-usage.vue(1 hunks)examples/sites/demos/pc/app/space/space-align.vue(1 hunks)examples/sites/demos/pc/app/space/space-direction.vue(1 hunks)examples/sites/demos/pc/app/space/space-justify.vue(1 hunks)examples/sites/demos/pc/app/space/space-order.vue(1 hunks)examples/sites/demos/pc/app/space/space-size.vue(1 hunks)examples/sites/demos/pc/app/space/space-wrap.vue(1 hunks)examples/sites/demos/pc/app/space/webdoc/space.cn.md(1 hunks)examples/sites/demos/pc/app/space/webdoc/space.en.md(1 hunks)examples/sites/demos/pc/app/space/webdoc/space.js(1 hunks)examples/sites/demos/pc/menus.js(1 hunks)packages/modules.json(1 hunks)packages/renderless/src/space/index.ts(1 hunks)packages/renderless/src/space/vue.ts(1 hunks)packages/renderless/types/index.ts(1 hunks)packages/renderless/types/space.type.ts(1 hunks)packages/vue/package.json(1 hunks)packages/vue/src/space/__tests__/space.test.tsx(1 hunks)packages/vue/src/space/index.ts(1 hunks)packages/vue/src/space/package.json(1 hunks)packages/vue/src/space/src/index.ts(1 hunks)packages/vue/src/space/src/mobile-first.vue(1 hunks)packages/vue/src/space/src/pc.vue(1 hunks)packages/vue/src/space/src/token.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/renderless/src/space/index.ts (1)
packages/renderless/types/space.type.ts (1)
ISpaceProps(14-23)
packages/vue/src/space/src/index.ts (1)
packages/vue-common/src/index.ts (4)
$props(45-53)$prefix(43-43)props(55-73)$setup(199-233)
packages/renderless/src/space/vue.ts (2)
packages/renderless/types/space.type.ts (1)
ISpaceProps(14-23)packages/renderless/src/space/index.ts (1)
getGapStyle(17-30)
⏰ 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 (19)
examples/sites/demos/mobile-first/app/space/space-order.vue (1)
2-7: Confirm intended behavior for unkeyed childThe fourth button lacks a key; with :order applied, only keyed children will be reordered and the unkeyed one keeps its natural position. Confirm that this is intentional for the demo narrative.
examples/sites/demos/mobile-first/app/space/space-direction.vue (1)
1-31: Direction toggle demo — looks goodBehavior and bindings are correct; concise example.
packages/vue/src/space/src/token.ts (1)
4-9: Verify handling of custom numeric/string gaps in production builds.Dynamic utility classes like
gap-[value]can be purged by the CSS pipeline if generated at runtime. Ensure non-preset sizes fall back to inline styles (e.g.,gapStyle) rather than relying solely on dynamic classes.packages/renderless/src/space/index.ts (1)
21-24: LGTM: correct mapping to CSS gap order.Input [horizontal, vertical] is mapped to "row-gap column-gap" =>
${vertical} ${horizontal}. This is correct.examples/sites/demos/pc/app/space/space-wrap.vue (1)
1-31: LGTM — demo is clear and works as intended.packages/vue/src/space/__tests__/space.test.tsx (1)
42-47: Target the container element for style assertions and confirm tuple ordering
Inprops sizetest, usewrapper.find('[data-tag="tiny-space"]')instead ofwrapper.attributes()to select the actual container. Verify whethersize={[10,20]}maps togap: row-gap column-gapor the reverse—current test comment and expected assertion conflict.examples/sites/demos/pc/app/space/space-direction.vue (1)
1-33: LGTM — concise demo for direction toggle.packages/modules.json (2)
2331-2349: Module registry entries for Space look correct.
Paths, types, and modes match existing conventions.
2331-2349: Registry keys and file references verified
All three “Space” entries appear exactly once in modules.json and their files exist.packages/vue/src/space/index.ts (1)
16-21: LGTM — plugin registration is standard for this repo.examples/sites/demos/pc/app/space/basic-usage.vue (1)
4-5: Unify direction values with API/docs (horizontal/vertical vs row/column).Docs in this PR use horizontal/vertical, while this demo uses row/column. Unless both are supported, this will confuse users and may not work.
Apply if the prop expects horizontal/vertical:
- <tiny-button type="primary" @click="setDirection('row')">行</tiny-button> - <tiny-button type="success" @click="setDirection('column')">列</tiny-button> + <tiny-button type="primary" @click="setDirection('horizontal')">行</tiny-button> + <tiny-button type="success" @click="setDirection('vertical')">列</tiny-button>- direction: 'column' // 初始方向为 column + direction: 'vertical' // 初始方向为 verticalAlso applies to: 28-29
examples/sites/demos/mobile-first/app/space/webdoc/space.js (1)
31-39: Align direction value names with demos (ensure the prop accepts the same vocabulary).This page says horizontal/vertical; some PC demos use row/column. Confirm accepted values and standardize across docs/demos.
examples/sites/demos/pc/app/space/space-align.vue (1)
1-14: LGTM for the demoWorks as expected. Note: this relies on TinySpace accepting 'start'/'end' values. Ensure pc.vue maps these to valid CSS ('flex-start'/'flex-end'); see my comment there.
examples/sites/demos/pc/app/space/space-justify.vue (1)
1-16: LGTM for the demoSame caveat as align: PC implementation should translate 'start'/'end' to 'flex-start'/'flex-end'.
packages/vue/src/space/package.json (3)
7-7: Double-check sideEffects=false with any style-only imports.If this package relies on CSS imported for side effects (e.g., theme CSS) inside this package, tree-shaking may drop them. Verify styles are imported from theme packages that handle their own sideEffects, or whitelist specific files.
8-9: Retain “module”: “index.ts”—consistent convention All vue packages inpackages/vue/src/**/package.jsonuse"module": "index.ts", so this is intentional and no change is required.
8-13: Invalid target – no package.json at packages/vue/src/space
That path does not contain a package.json; exports and types entries belong in the actual package manifest (e.g. packages/vue/package.json).Likely an incorrect or invalid review comment.
packages/vue/src/space/src/index.ts (2)
33-36: Border default = true: confirm UX and compatibility.Space containers typically default to no border. If previous components defaulted to false, this could be surprising. Confirm design intent and that this isn’t a breaking default.
65-67: SSR/build modes: sanity check virtual-template resolution.Ensure the
virtual-template?pc|mobile-firstplugin resolves in SSR and test environments (Vitest) without conditional import issues. Add a basic SSR test if applicable.
| name: 'order', | ||
| type: 'array', | ||
| defaultValue: '[]', | ||
| desc: { | ||
| 'zh-CN': '用于指定子元素的显示顺序，数组的值对应插槽中子节点的索引', | ||
| 'en-US': | ||
| 'Used to specify the display order of child elements; the array values correspond to the index of slot children' | ||
| }, |
There was a problem hiding this comment.
Mismatch: docs say “index of slot children” but demos use vnode keys. Pick one and align.
If reordering is by vnode key (recommended), update docs accordingly; if by index, update demos to use indices (and remove reliance on key). This is user-facing and will confuse consumers.
Suggested doc update if using keys:
- 'en-US':
- 'Used to specify the display order of child elements; the array values correspond to the index of slot children'
+ 'en-US':
+ 'Used to specify the display order of child elements; the array values correspond to each child’s vnode key'If using indices instead, I can provide a revised demo using v-for with order like [1, 2, 0]. Prefer which?
📝 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.
| name: 'order', | |
| type: 'array', | |
| defaultValue: '[]', | |
| desc: { | |
| 'zh-CN': '用于指定子元素的显示顺序，数组的值对应插槽中子节点的索引', | |
| 'en-US': | |
| 'Used to specify the display order of child elements; the array values correspond to the index of slot children' | |
| }, | |
| name: 'order', | |
| type: 'array', | |
| defaultValue: '[]', | |
| desc: { | |
| 'zh-CN': '用于指定子元素的显示顺序，数组的值对应插槽中子节点的索引', | |
| 'en-US': | |
| 'Used to specify the display order of child elements; the array values correspond to each child’s vnode key' | |
| }, |
🤖 Prompt for AI Agents
In examples/sites/demos/apis/space.js around lines 69 to 76, the prop "order"
documentation says it uses the index of slot children but the demo
implementation uses vnode keys; pick one and make docs and demo consistent. If
you choose vnode keys (recommended), update both 'zh-CN' and 'en-US' desc to
state that order contains vnode keys used to reorder children and leave demo
as-is; if you choose indices, change defaultValue to an array of numbers, update
the demo to derive ordering by child indices (remove reliance on vnode keys) and
adjust both locale descriptions to say indices instead of keys.
| <tiny-button key="1">First Button</tiny-button> | ||
| <tiny-button key="2">Second Button</tiny-button> | ||
| <tiny-button key="3">Third Button</tiny-button> | ||
| <tiny-button>Forth Button</tiny-button> |
There was a problem hiding this comment.
Typo: “Forth Button” → “Fourth Button”.
User-facing text.
- <tiny-button>Forth Button</tiny-button>
+ <tiny-button>Fourth Button</tiny-button>📝 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.
| <tiny-button>Forth Button</tiny-button> | |
| <tiny-button>Fourth Button</tiny-button> |
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/space/space-order.vue around line 6, the
user-facing label "Forth Button" is misspelled; change the button text to
"Fourth Button" (i.e., replace the inner text of the <tiny-button> from "Forth
Button" to "Fourth Button") and save the template; if project uses i18n or
strings elsewhere, update the corresponding resource key instead of hardcoding.
| <tiny-space :size="[rowValue, columnValue]" :wrap="true"> | ||
| <tiny-button style="margin: 0" v-for="n in 15" :key="n">按钮 {{ n }}</tiny-button> | ||
| </tiny-space> |
There was a problem hiding this comment.
Fix row/column gap mapping — current binding reverses CSS row-gap/column-gap.
getGapStyle treats size as [horizontal, vertical] but CSS gap is "row column". To make “行(row)” control row-gap and “列(column)” control column-gap, pass [columnValue, rowValue].
- <tiny-space :size="[rowValue, columnValue]" :wrap="true">
+ <tiny-space :size="[columnValue, rowValue]" :wrap="true">📝 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.
| <tiny-space :size="[rowValue, columnValue]" :wrap="true"> | |
| <tiny-button style="margin: 0" v-for="n in 15" :key="n">按钮 {{ n }}</tiny-button> | |
| </tiny-space> | |
| <tiny-space :size="[columnValue, rowValue]" :wrap="true"> | |
| <tiny-button style="margin: 0" v-for="n in 15" :key="n">按钮 {{ n }}</tiny-button> | |
| </tiny-space> |
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/space/space-size.vue around lines 17 to 19, the
gap binding passes [rowValue, columnValue] which reverses row/column mapping vs
CSS; change the prop to pass [columnValue, rowValue] so the first element maps
to CSS column-gap and the second to row-gap (i.e., swap the two values in the
array) ensuring “行(row)” controls row-gap and “列(column)” controls column-gap.
| "dependencies": { | ||
| "@opentiny/vue-common": "workspace:~", | ||
| "@opentiny/vue-icon": "workspace:~", | ||
| "@opentiny/vue-renderless": "workspace:~", | ||
| "@opentiny/vue-theme": "workspace:~" | ||
| }, | ||
| "devDependencies": { | ||
| "@opentiny-internal/vue-test-utils": "workspace:*", | ||
| "vitest": "catalog:" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Declare Vue as a peer dependency to prevent duplicate Vue instances.
Component packages typically declare vue as a peer to align with app-level Vue. Add a peer (and optional peer meta).
"dependencies": {
"@opentiny/vue-common": "workspace:~",
"@opentiny/vue-icon": "workspace:~",
"@opentiny/vue-renderless": "workspace:~",
"@opentiny/vue-theme": "workspace:~"
},
+ "peerDependencies": {
+ "vue": "^3.3.0"
+ },
+ "peerDependenciesMeta": {
+ "vue": { "optional": false }
+ },📝 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.
| "dependencies": { | |
| "@opentiny/vue-common": "workspace:~", | |
| "@opentiny/vue-icon": "workspace:~", | |
| "@opentiny/vue-renderless": "workspace:~", | |
| "@opentiny/vue-theme": "workspace:~" | |
| }, | |
| "devDependencies": { | |
| "@opentiny-internal/vue-test-utils": "workspace:*", | |
| "vitest": "catalog:" | |
| } | |
| "dependencies": { | |
| "@opentiny/vue-common": "workspace:~", | |
| "@opentiny/vue-icon": "workspace:~", | |
| "@opentiny/vue-renderless": "workspace:~", | |
| "@opentiny/vue-theme": "workspace:~" | |
| }, | |
| "peerDependencies": { | |
| "vue": "^3.3.0" | |
| }, | |
| "peerDependenciesMeta": { | |
| "vue": { "optional": false } | |
| }, | |
| "devDependencies": { | |
| "@opentiny-internal/vue-test-utils": "workspace:*", | |
| "vitest": "catalog:" | |
| } |
🤖 Prompt for AI Agents
packages/vue/src/space/package.json lines 14-23: this package currently only
lists internal packages in dependencies/devDependencies but does not declare Vue
as a peer, which can cause duplicate Vue instances; add a peerDependencies entry
for "vue" (use the same major/version range as the repo/root package, e.g. "^3")
and optionally add peerDependenciesMeta for "vue" (or set optional false) to
make intent explicit and ensure consumers provide a single shared Vue instance.
| class: m( | ||
| gcls('base'), | ||
| gcls(`direction-${this.direction || 'row'}`), | ||
| gcls(`align-${this.align || 'start'}`), | ||
| gcls(`justify-${this.justify || 'start'}`), | ||
| gcls(`wrap-${this.wrap ? 'true' : 'false'}`) | ||
| ), | ||
| style: this.state?.gapStyle | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply customClass/customStyle for consistency with PC and public API.
Props exist but aren’t used; wire them through.
- {
- 'data-tag': 'tiny-space',
- class: m(
+ {
+ 'data-tag': 'tiny-space',
+ class: m(
gcls('base'),
gcls(`direction-${this.direction || 'row'}`),
gcls(`align-${this.align || 'start'}`),
gcls(`justify-${this.justify || 'start'}`),
- gcls(`wrap-${this.wrap ? 'true' : 'false'}`)
- ),
- style: this.state?.gapStyle
+ gcls(`wrap-${this.wrap ? 'true' : 'false'}`),
+ this.customClass
+ ),
+ style: [this.state?.gapStyle, this.customStyle]
},📝 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.
| class: m( | |
| gcls('base'), | |
| gcls(`direction-${this.direction || 'row'}`), | |
| gcls(`align-${this.align || 'start'}`), | |
| gcls(`justify-${this.justify || 'start'}`), | |
| gcls(`wrap-${this.wrap ? 'true' : 'false'}`) | |
| ), | |
| style: this.state?.gapStyle | |
| }, | |
| { | |
| 'data-tag': 'tiny-space', | |
| class: m( | |
| gcls('base'), | |
| gcls(`direction-${this.direction || 'row'}`), | |
| gcls(`align-${this.align || 'start'}`), | |
| gcls(`justify-${this.justify || 'start'}`), | |
| gcls(`wrap-${this.wrap ? 'true' : 'false'}`), | |
| this.customClass | |
| ), | |
| style: [this.state?.gapStyle, this.customStyle] | |
| }, |
🤖 Prompt for AI Agents
In packages/vue/src/space/src/mobile-first.vue around lines 21 to 29, the
component builds class and style from internal helpers but ignores the public
props customClass and customStyle; update the class to include this.customClass
(merged into the m(...) call or concatenated after gcls entries) and update the
style to merge this.state?.gapStyle with this.customStyle (ensuring neither is
mutated and handling undefined). Ensure the merged class and style preserve
existing computed classes and gapStyle while giving callers ability to pass
additional classes/styles, and use safe checks so undefined/null props are
ignored.
| children.map((child, idx) => | ||
| typeof child === 'function' ? child() : h(child.type || child, { key: child.key ?? idx, ...child.props }) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Guard spreading of child.props to avoid runtime TypeError.
child.props can be undefined/null on some VNodes.
- children.map((child, idx) =>
- typeof child === 'function' ? child() : h(child.type || child, { key: child.key ?? idx, ...child.props })
- )
+ children.map((child, idx) =>
+ typeof child === 'function'
+ ? child()
+ : h(child.type || child, { key: child.key ?? idx, ...(child?.props || {}) })
+ )📝 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.
| children.map((child, idx) => | |
| typeof child === 'function' ? child() : h(child.type || child, { key: child.key ?? idx, ...child.props }) | |
| ) | |
| ) | |
| children.map((child, idx) => | |
| typeof child === 'function' | |
| ? child() | |
| : h(child.type || child, { key: child.key ?? idx, ...(child?.props || {}) }) | |
| ) |
🤖 Prompt for AI Agents
In packages/vue/src/space/src/mobile-first.vue around lines 30 to 33, the code
spreads child.props directly which can be undefined/null and cause a runtime
TypeError; update the props spread to guard against falsy props (for example
spread an empty object when child.props is nullish or conditionally include
props) and ensure you still include key and other explicit props—i.e. replace
the direct "...child.props" with a safe expression like "...(child.props ?? {})"
or a conditional merge so rendering never attempts to spread undefined.
| render() { | ||
| const hFunc = (this as any).$createElement || h | ||
|
|
||
| // 如果 renderless 层有 orderedChildren，就用它，否则 fallback 到默认 slot | ||
| const children = this.orderedChildren?.length ? this.orderedChildren : this.$slots.default?.() || [] | ||
|
|
||
| return hFunc( | ||
| 'div', | ||
| { | ||
| class: this.customClass, | ||
| style: [ | ||
| this.state?.gapStyle, | ||
| { | ||
| display: 'flex', | ||
| flexDirection: this.direction || 'row', | ||
| alignItems: this.align || 'flex-start', | ||
| justifyContent: this.justify || 'flex-start', | ||
| flexWrap: this.wrap ? 'wrap' : 'nowrap', | ||
| ...this.customStyle | ||
| } | ||
| ] | ||
| }, | ||
| children | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Translate 'start'/'end' to valid CSS and add data-tag; also normalize children for parity with mobile-first.
Without translation, alignItems/justifyContent receive non-standard values; adding data-tag keeps parity with MF/tests. Normalizing children handles function VNodes and adds stable keys.
render() {
const hFunc = (this as any).$createElement || h
-
- // 如果 renderless 层有 orderedChildren，就用它，否则 fallback 到默认 slot
- const children = this.orderedChildren?.length ? this.orderedChildren : this.$slots.default?.() || []
+ // 如果 renderless 层有 orderedChildren，就用它，否则 fallback 到默认 slot
+ const toFlexPos = (v?: string) => (v === 'start' ? 'flex-start' : v === 'end' ? 'flex-end' : v)
+ const rawChildren = this.orderedChildren?.length ? this.orderedChildren : this.$slots.default?.() || []
+ const children = rawChildren.map((child: any, idx: number) =>
+ typeof child === 'function' ? child() : hFunc(child.type || child, { key: child?.key ?? idx, ...(child?.props || {}) })
+ )
return hFunc(
'div',
{
- class: this.customClass,
+ 'data-tag': 'tiny-space',
+ class: this.customClass,
style: [
this.state?.gapStyle,
{
display: 'flex',
flexDirection: this.direction || 'row',
- alignItems: this.align || 'flex-start',
- justifyContent: this.justify || 'flex-start',
+ alignItems: toFlexPos(this.align) || 'flex-start',
+ justifyContent: toFlexPos(this.justify) || 'flex-start',
flexWrap: this.wrap ? 'wrap' : 'nowrap',
...this.customStyle
}
]
},
children
)
}📝 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.
| render() { | |
| const hFunc = (this as any).$createElement || h | |
| // 如果 renderless 层有 orderedChildren，就用它，否则 fallback 到默认 slot | |
| const children = this.orderedChildren?.length ? this.orderedChildren : this.$slots.default?.() || [] | |
| return hFunc( | |
| 'div', | |
| { | |
| class: this.customClass, | |
| style: [ | |
| this.state?.gapStyle, | |
| { | |
| display: 'flex', | |
| flexDirection: this.direction || 'row', | |
| alignItems: this.align || 'flex-start', | |
| justifyContent: this.justify || 'flex-start', | |
| flexWrap: this.wrap ? 'wrap' : 'nowrap', | |
| ...this.customStyle | |
| } | |
| ] | |
| }, | |
| children | |
| ) | |
| } | |
| render() { | |
| const hFunc = (this as any).$createElement || h | |
| // 如果 renderless 层有 orderedChildren，就用它，否则 fallback 到默认 slot | |
| const toFlexPos = (v?: string) => (v === 'start' ? 'flex-start' : v === 'end' ? 'flex-end' : v) | |
| const rawChildren = this.orderedChildren?.length ? this.orderedChildren : this.$slots.default?.() || [] | |
| const children = rawChildren.map((child: any, idx: number) => | |
| typeof child === 'function' | |
| ? child() | |
| : hFunc(child.type || child, { key: child?.key ?? idx, ...(child?.props || {}) }) | |
| ) | |
| return hFunc( | |
| 'div', | |
| { | |
| 'data-tag': 'tiny-space', | |
| class: this.customClass, | |
| style: [ | |
| this.state?.gapStyle, | |
| { | |
| display: 'flex', | |
| flexDirection: this.direction || 'row', | |
| alignItems: toFlexPos(this.align) || 'flex-start', | |
| justifyContent: toFlexPos(this.justify) || 'flex-start', | |
| flexWrap: this.wrap ? 'wrap' : 'nowrap', | |
| ...this.customStyle | |
| } | |
| ] | |
| }, | |
| children | |
| ) | |
| } |
🤖 Prompt for AI Agents
In packages/vue/src/space/src/pc.vue around lines 47 to 71, translate
align/start and end values to valid CSS (map 'start' -> 'flex-start' and 'end'
-> 'flex-end' before setting alignItems/justifyContent), add a data-tag
attribute on the root div for parity with mobile/tests, and normalize children
before passing to the render function by resolving function VNodes (invoke if a
child is a function), flattening/ensuring an array, and assigning stable keys
where missing (e.g., using the index) so orderedChildren fallback uses
normalized VNodes.
| // flex-direction | ||
| 'direction-row': 'flex-row', | ||
| 'direction-row-reverse': 'flex-row-reverse', | ||
| 'direction-column': 'flex-col', | ||
| 'direction-column-reverse': 'flex-col-reverse', |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Tokens expose reverse directions; types don't.
You map direction-row-reverse and direction-column-reverse here, but the public SpaceDirection type omits them. Either remove these tokens or add the values to the type to avoid unreachable code paths.
🤖 Prompt for AI Agents
In packages/vue/src/space/src/token.ts around lines 10 to 14, the token map
includes 'direction-row-reverse' and 'direction-column-reverse' but the public
SpaceDirection type omits those values causing unreachable code paths; either
remove those reverse direction entries from the token map or (preferred) update
the SpaceDirection type definition to include 'direction-row-reverse' and
'direction-column-reverse' so the tokens and type align, then run type checks to
ensure no other code relies on the original narrower type.
|
@ynnnny 代码有冲突哈 |
@ynnnny The code conflicts |
PR
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
New Features
Documentation
Tests
Chores