-
Notifications
You must be signed in to change notification settings - Fork 273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(steps): [steps] The stateless step style is added to the step bar, and the itemFooter slot is added. #2110
Conversation
WalkthroughThis pull request introduces several enhancements to the steps component in a Vue.js application. Key changes include the addition of an Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range and nitpick comments (3)
examples/sites/demos/pc/app/steps/slot-item-footer-composition-api.vue (1)
1-57
: Potential improvements.The component looks good overall, but here are some potential improvements to consider:
- Extract the
itemFooter
slot into a separate component for better readability and maintainability. This will make the component easier to understand and maintain.- Use more descriptive names for the
stepsData
andhandleChange
variables and method. This will make the component easier to understand and maintain.- Use TypeScript for better type safety and documentation. This will make the component easier to understand and maintain, and will help catch potential bugs early.
packages/vue/src/steps/src/pc/pc-line.vue (1)
92-92
: Consider adjusting the tooltip placement for the vertical title.While using the
v-auto-tip
directive for the vertical title is a good enhancement, the 'bottom' placement might not be the most suitable as it could overlap with the content below.Consider changing the placement to 'right' for a better user experience:
-<div v-auto-tip="{ placement: 'bottom' }" :class="['title-vertical', { 'not-vertical': !vertical }]"> +<div v-auto-tip="{ placement: 'right' }" :class="['title-vertical', { 'not-vertical': !vertical }]">examples/sites/demos/apis/steps.js (1)
224-238
: LGTM! Consider adding a mobile-first demo if applicable.The addition of the
itemFooter
slot looks good and follows the existing code conventions. The naming is clear, the typing is consistent, and the documentation provides helpful information about the slot's purpose and usage.The inclusion of the stability version in the
meta
property is a nice touch for tracking changes.One suggestion would be to consider adding a mobile-first demo (
mfDemo
) if this slot is also applicable in mobile environments. If not, the current 'pc' mode specification is sufficient.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- examples/sites/demos/apis/steps.js (1 hunks)
- examples/sites/demos/pc/app/steps/slot-item-footer-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/steps/slot-item-footer.vue (1 hunks)
- examples/sites/demos/pc/app/steps/webdoc/steps.js (2 hunks)
- packages/theme/src/steps/index.less (4 hunks)
- packages/vue-directive/src/auto-tip.ts (1 hunks)
- packages/vue/src/steps/package.json (1 hunks)
- packages/vue/src/steps/src/pc/pc-line.vue (5 hunks)
Additional comments not posted (24)
packages/vue/src/steps/package.json (5)
3-3
: LGTM!Adding the
"type": "module"
field is a good practice as it explicitly indicates that the code in the package is written as an ECMAScript module. This clarifies the module system used by the package.
6-6
: LGTM!Adding the
"license": "MIT"
field is important for open-source projects. The MIT license is a permissive open-source license that allows users to freely use, modify, and distribute the code.
7-7
: LGTM!Setting
"sideEffects": false
is beneficial as it indicates that the package does not have any side effects when imported. This allows bundlers to perform tree-shaking and eliminate unused code, resulting in smaller bundle sizes.
22-25
: Verify the necessity and compatibility of the added development dependencies.The reintroduction of the
devDependencies
section with the@opentiny-internal/vue-test-utils
andvitest
packages indicates a shift in how development dependencies are managed. Please ensure that these packages are necessary for the package's development and testing workflow and are compatible with the existing setup.Run the following script to verify the usage of the added development dependencies in the codebase:
Verification successful
Development dependencies are necessary and compatible.
The
@opentiny-internal/vue-test-utils
andvitest
packages are actively used in the codebase, as evidenced by their import statements in TypeScript files. This confirms their necessity and compatibility with the existing development and testing workflow. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the added development dependencies in the codebase. # Test: Search for the import statements of the added development dependencies. # Expect: At least one occurrence of each development dependency import. fd --type ts --exec rg --multiline $'import.*@opentiny-internal/vue-test-utils' fd --type ts --exec rg --multiline $'import.*vitest'Length of output: 474
Script:
#!/bin/bash # Description: Verify the usage of the added development dependencies in the codebase. # Find all TypeScript files and search for import statements of the added development dependencies. fd --extension ts --exec rg --multiline 'import.*@opentiny-internal/vue-test-utils' {} fd --extension ts --exec rg --multiline 'import.*vitest' {}Length of output: 559
16-17
: Verify the necessity and compatibility of the added dependencies.Adding new dependencies is a significant change that can impact the package's functionality and compatibility. Please ensure that the
@opentiny/vue-directive
and@opentiny/vue-icon
packages are necessary for the functionality introduced in this PR and are compatible with the package's existing dependencies and the project's overall ecosystem.Run the following script to verify the usage of the added dependencies in the codebase:
examples/sites/demos/pc/app/steps/slot-item-footer-composition-api.vue (2)
1-19
: LGTM!The template section looks good:
- The
tiny-steps
andtiny-link
components are used correctly.- The
itemFooter
slot is used correctly to customize the footer of each step item.- The
v-if
andv-for
directives are used correctly to conditionally render the links and multipletiny-link
components.- The
#icon
slot is used correctly to conditionally render the icon component.
53-57
: LGTM!The style section looks good:
- The style is scoped correctly to the component.
- The style adds a left margin of 16px to
.item-link
elements that are not the first child, which looks correct.examples/sites/demos/pc/app/steps/slot-item-footer.vue (2)
1-19
: LGTM!The template section correctly demonstrates the usage of the
tiny-steps
component with a customitemFooter
slot. The rendering of thetiny-link
component for each link and the conditional rendering of the link's icon are implemented correctly.
65-69
: LGTM!The scoped style correctly adds a left margin of 16px to
.item-link
elements that are preceded by another.item-link
element. Using a scoped style is a good practice to avoid style conflicts with other components.packages/vue-directive/src/auto-tip.ts (1)
34-35
: LGTM!The updated condition in the
isEllipsis
function correctly checks for both horizontal and vertical overflow, improving the tooltip display mechanism. This change aligns with the PR objective and is implemented correctly.examples/sites/demos/pc/app/steps/webdoc/steps.js (2)
112-112
: LGTM!The added space improves the readability of the Chinese name. Good catch!
121-132
: Excellent addition!The new demo entry for the
itemFooter
slot is a great addition to the documentation. The names and descriptions are clear and consistent with the existing entries. The example provided in the description effectively illustrates the usage of the slot.packages/vue/src/steps/src/pc/pc-line.vue (5)
44-45
: LGTM!The conditional classes are correctly applied based on the number of items and orientation to adjust the styling.
77-77
: Great addition!Using the
v-auto-tip
directive to display tooltips on the title is a nice enhancement to provide additional context to the user. The 'bottom' placement is suitable for the horizontal title.
103-103
: LGTM!Adding the
v-auto-tip
directive to the description is consistent with the other tooltip enhancements. The 'bottom' placement is suitable for the description.
107-109
: Nice work!The new
itemFooter
slot is a great addition to enhance the flexibility of the component by allowing users to render additional content at the bottom of each item. The slot is correctly scoped and conditionally rendered based on the presence of content.
146-146
: LGTM!Importing the
AutoTip
directive from@opentiny/vue-directive
and registering it in the component using the new syntax is necessary and correctly done to enable the usage of thev-auto-tip
directive in the template.Also applies to: 153-153
packages/theme/src/steps/index.less (7)
372-373
: LGTM!The margin adjustments for
.description
and.line-footer
elements in the vertical layout look good. This ensures consistent spacing and improves the visual alignment.
587-593
: Looks good!The new
.block-mini
class is a nice addition for creating more compact layouts. Restricting the maximum width of.title
,.description
, and.line-footer
elements to 150px within this class is a reasonable choice.
595-600
: Approved.Setting a default maximum width of 250px for
.title
,.description
, and.line-footer
elements is a good practice. It helps maintain a consistent and readable layout by preventing the content from extending too wide. The chosen value of 250px seems appropriate for most cases.
604-610
: Great improvements to the title text handling!The changes made to the
.title
class significantly enhance its visual appearance and consistency:
- Setting the line height to 1.5 improves readability.
- Using
-webkit-line-clamp: 2
limits the title to a maximum of 2 lines, maintaining a consistent height.- Allowing word breaking ensures proper wrapping of long words within the available width.
These improvements contribute to a better overall layout and user experience. Well done!
657-661
: Approved.The line height and margin adjustments for
.description
and.line-footer
elements are beneficial:
- Setting the line height to 1.5 improves readability and maintains consistent vertical spacing.
- Calculating the left margin based on the step circle width and a predefined value ensures proper alignment with the step circle.
These changes enhance the overall layout and visual appeal of the step components. Good job!
682-682
: LGTM.Adjusting the top margin of the
.description
element to 10px in the non-vertical layout is a good choice. It creates consistent vertical spacing between the title and description, improving the visual separation and readability of the step content in this layout.
686-688
: Nice addition!Introducing styles for the
.line-footer
element is a good move to maintain consistent styling across the step components. Setting a top margin of 8px for the.line-footer
creates an appropriate vertical spacing, improving the visual separation and hierarchy within the step layout. Well done!
<script setup> | ||
import { ref } from 'vue' | ||
import { Button as TinyButton, Steps as TinySteps, Link as TinyLink } from '@opentiny/vue' | ||
import { IconFilletExternalLink } from '@opentiny/vue-icon' | ||
|
||
const stepsData = ref([ | ||
{ | ||
name: '创建桶(超长标题超长标题超长标题超长标题超长标题超长标题超长标题超长标题超长标题)', | ||
status: 'done', | ||
description: | ||
'桶是存储对象的容器,用于存储您的数据,我这个描述要超出隐藏,我这个描述要超出隐藏,我这个描述要超出隐藏', | ||
links: [{ title: '前往创建' }, { title: '前往创建' }] | ||
}, | ||
{ name: '上传文件', status: 'doing', description: '上传您项目相关的文件至桶内存储' }, | ||
{ | ||
name: '购买资源包(可选)', | ||
description: '资源包是预付费的优惠套餐,可降低存储成本。', | ||
links: [{ title: '了解套餐详情', icon: IconFilletExternalLink() }] | ||
} | ||
]) | ||
const handleChange = () => { | ||
if (stepsData.value.length > 3) { | ||
stepsData.value.pop() | ||
} else { | ||
stepsData.value.push({ | ||
name: '域名解析', | ||
description: '域名解析的内容' | ||
}) | ||
} | ||
} | ||
</script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use immutable update patterns in the handleChange
method.
The script section looks good overall:
- The imports are correct.
- The
stepsData
ref is defined correctly with an array of step objects. - The
handleChange
method logic is correct. It checks the length of thestepsData
array and adds or removes a step accordingly.
However, the handleChange
method mutates the stepsData
array directly. This is generally not recommended in Vue 3. It's better to use immutable update patterns.
Consider using the spread operator to create a new array instead of mutating the existing one:
const handleChange = () => {
if (stepsData.value.length > 3) {
- stepsData.value.pop()
+ stepsData.value = stepsData.value.slice(0, -1)
} else {
- stepsData.value.push({
+ stepsData.value = [...stepsData.value, {
name: '域名解析',
description: '域名解析的内容'
- })
+ }]
}
}
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.
<script setup> | |
import { ref } from 'vue' | |
import { Button as TinyButton, Steps as TinySteps, Link as TinyLink } from '@opentiny/vue' | |
import { IconFilletExternalLink } from '@opentiny/vue-icon' | |
const stepsData = ref([ | |
{ | |
name: '创建桶(超长标题超长标题超长标题超长标题超长标题超长标题超长标题超长标题超长标题)', | |
status: 'done', | |
description: | |
'桶是存储对象的容器,用于存储您的数据,我这个描述要超出隐藏,我这个描述要超出隐藏,我这个描述要超出隐藏', | |
links: [{ title: '前往创建' }, { title: '前往创建' }] | |
}, | |
{ name: '上传文件', status: 'doing', description: '上传您项目相关的文件至桶内存储' }, | |
{ | |
name: '购买资源包(可选)', | |
description: '资源包是预付费的优惠套餐,可降低存储成本。', | |
links: [{ title: '了解套餐详情', icon: IconFilletExternalLink() }] | |
} | |
]) | |
const handleChange = () => { | |
if (stepsData.value.length > 3) { | |
stepsData.value.pop() | |
} else { | |
stepsData.value.push({ | |
name: '域名解析', | |
description: '域名解析的内容' | |
}) | |
} | |
} | |
</script> | |
<script setup> | |
import { ref } from 'vue' | |
import { Button as TinyButton, Steps as TinySteps, Link as TinyLink } from '@opentiny/vue' | |
import { IconFilletExternalLink } from '@opentiny/vue-icon' | |
const stepsData = ref([ | |
{ | |
name: '创建桶(超长标题超长标题超长标题超长标题超长标题超长标题超长标题超长标题超长标题)', | |
status: 'done', | |
description: | |
'桶是存储对象的容器,用于存储您的数据,我这个描述要超出隐藏,我这个描述要超出隐藏,我这个描述要超出隐藏', | |
links: [{ title: '前往创建' }, { title: '前往创建' }] | |
}, | |
{ name: '上传文件', status: 'doing', description: '上传您项目相关的文件至桶内存储' }, | |
{ | |
name: '购买资源包(可选)', | |
description: '资源包是预付费的优惠套餐,可降低存储成本。', | |
links: [{ title: '了解套餐详情', icon: IconFilletExternalLink() }] | |
} | |
]) | |
const handleChange = () => { | |
if (stepsData.value.length > 3) { | |
stepsData.value = stepsData.value.slice(0, -1) | |
} else { | |
stepsData.value = [...stepsData.value, { | |
name: '域名解析', | |
description: '域名解析的内容' | |
}] | |
} | |
} | |
</script> |
<script> | ||
import { Button, Steps, Link } from '@opentiny/vue' | ||
import { IconFilletExternalLink } from '@opentiny/vue-icon' | ||
|
||
export default { | ||
components: { | ||
TinyButton: Button, | ||
TinySteps: Steps, | ||
TinyLink: Link | ||
}, | ||
data() { | ||
return { | ||
stepsData: [ | ||
{ | ||
name: '创建桶(超长标题超长标题超长标题超长标题超长标题超长标题超长标题超长标题超长标题)', | ||
status: 'done', | ||
description: | ||
'桶是存储对象的容器,用于存储您的数据,我这个描述要超出隐藏,我这个描述要超出隐藏,我这个描述要超出隐藏', | ||
links: [{ title: '前往创建' }, { title: '前往创建' }] | ||
}, | ||
{ name: '上传文件', status: 'doing', description: '上传您项目相关的文件至桶内存储' }, | ||
{ | ||
name: '购买资源包(可选)', | ||
description: '资源包是预付费的优惠套餐,可降低存储成本。', | ||
links: [{ title: '了解套餐详情', icon: IconFilletExternalLink() }] | ||
} | ||
] | ||
} | ||
}, | ||
methods: { | ||
handleChange() { | ||
if (this.stepsData.length > 3) { | ||
this.stepsData.pop() | ||
} else { | ||
this.stepsData.push({ | ||
name: '域名解析', | ||
description: '域名解析的内容' | ||
}) | ||
} | ||
} | ||
} | ||
} | ||
</script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script section looks good! Consider a minor refactor.
The component imports, components
option, data
property, and handleChange
method are all implemented correctly.
As a minor refactor suggestion, consider using array spread syntax in the handleChange
method when adding a new step to stepsData
to make it more concise:
handleChange() {
if (this.stepsData.length > 3) {
this.stepsData.pop()
} else {
- this.stepsData.push({
+ this.stepsData = [...this.stepsData, {
name: '域名解析',
description: '域名解析的内容'
- })
+ }]
}
}
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.
<script> | |
import { Button, Steps, Link } from '@opentiny/vue' | |
import { IconFilletExternalLink } from '@opentiny/vue-icon' | |
export default { | |
components: { | |
TinyButton: Button, | |
TinySteps: Steps, | |
TinyLink: Link | |
}, | |
data() { | |
return { | |
stepsData: [ | |
{ | |
name: '创建桶(超长标题超长标题超长标题超长标题超长标题超长标题超长标题超长标题超长标题)', | |
status: 'done', | |
description: | |
'桶是存储对象的容器,用于存储您的数据,我这个描述要超出隐藏,我这个描述要超出隐藏,我这个描述要超出隐藏', | |
links: [{ title: '前往创建' }, { title: '前往创建' }] | |
}, | |
{ name: '上传文件', status: 'doing', description: '上传您项目相关的文件至桶内存储' }, | |
{ | |
name: '购买资源包(可选)', | |
description: '资源包是预付费的优惠套餐,可降低存储成本。', | |
links: [{ title: '了解套餐详情', icon: IconFilletExternalLink() }] | |
} | |
] | |
} | |
}, | |
methods: { | |
handleChange() { | |
if (this.stepsData.length > 3) { | |
this.stepsData.pop() | |
} else { | |
this.stepsData.push({ | |
name: '域名解析', | |
description: '域名解析的内容' | |
}) | |
} | |
} | |
} | |
} | |
</script> | |
<script> | |
import { Button, Steps, Link } from '@opentiny/vue' | |
import { IconFilletExternalLink } from '@opentiny/vue-icon' | |
export default { | |
components: { | |
TinyButton: Button, | |
TinySteps: Steps, | |
TinyLink: Link | |
}, | |
data() { | |
return { | |
stepsData: [ | |
{ | |
name: '创建桶(超长标题超长标题超长标题超长标题超长标题超长标题超长标题超长标题超长标题)', | |
status: 'done', | |
description: | |
'桶是存储对象的容器,用于存储您的数据,我这个描述要超出隐藏,我这个描述要超出隐藏,我这个描述要超出隐藏', | |
links: [{ title: '前往创建' }, { title: '前往创建' }] | |
}, | |
{ name: '上传文件', status: 'doing', description: '上传您项目相关的文件至桶内存储' }, | |
{ | |
name: '购买资源包(可选)', | |
description: '资源包是预付费的优惠套餐,可降低存储成本。', | |
links: [{ title: '了解套餐详情', icon: IconFilletExternalLink() }] | |
} | |
] | |
} | |
}, | |
methods: { | |
handleChange() { | |
if (this.stepsData.length > 3) { | |
this.stepsData.pop() | |
} else { | |
this.stepsData = [...this.stepsData, { | |
name: '域名解析', | |
description: '域名解析的内容' | |
}] | |
} | |
} | |
} | |
} | |
</script> |
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
Release Notes
New Features
itemFooter
property for enhanced step bar functionality.Bug Fixes
Styling Enhancements
Documentation
itemFooter
slot with clear customization instructions.