Skip to content
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: [config-provider] optimize global configuration component documentation #2578

Merged
merged 3 commits into from
Nov 30, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,20 +1,54 @@
<template>
<div>
<tiny-config-provider :design="design">
<tiny-alert type="warning" description="type 为 warning"></tiny-alert>
<div class="demo-form">
<tiny-alert type="warning" description="全局配置组件的默认行为"></tiny-alert>
<tiny-form ref="ruleFormRef" :model="formData">
<tiny-form-item label="年龄" prop="age" required>
<tiny-numeric v-model="formData.age"></tiny-numeric>
</tiny-form-item>
<tiny-form-item label="姓名" prop="name" required>
<tiny-input v-model="formData.name"></tiny-input>
</tiny-form-item>
<tiny-form-item>
<tiny-button @click="handleSubmitPromise" type="primary"> 校验 </tiny-button>
</tiny-form-item>
</tiny-form>
</div>
</tiny-config-provider>
</div>
</template>

<script setup>
import { ref } from 'vue'
import { TinyConfigProvider, TinyAlert, TinyModal } from '@opentiny/vue'
import {
TinyConfigProvider,
TinyAlert,
TinyModal,
TinyForm,
TinyFormItem,
TinyInput,
TinyNumeric,
TinyButton
} from '@opentiny/vue'
import { iconWarningTriangle } from '@opentiny/vue-icon'

const design = ref({
name: 'smb', // 设计规范名称
const ruleFormRef = ref()
const design = {
name: 'x-design', // 设计规范名称
version: '1.0.0', // 设计规范版本号
components: {
Form: {
props: {
hideRequiredAsterisk: true
}
},
Button: {
props: {
resetTime: 0,
round: true
}
},
Alert: {
icons: {
warning: iconWarningTriangle()
Expand All @@ -38,5 +72,20 @@ const design = ref({
}
}
}
}

const handleSubmitPromise = () => {
ruleFormRef.value.validate()
}

const formData = ref({
name: '',
age: ''
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve form validation handling and data initialization

The current implementation has two issues:

  1. The validation result is not handled
  2. Age field is initialized with an empty string instead of null/undefined
-const handleSubmitPromise = () => {
-  ruleFormRef.value.validate()
+const handleSubmitPromise = async () => {
+  try {
+    await ruleFormRef.value.validate()
+    // Handle successful validation
+    console.log('Form validated successfully:', formData.value)
+  } catch (error) {
+    console.error('Validation failed:', error)
+  }
}

const formData = ref({
  name: '',
-  age: ''
+  age: null
})
📝 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.

Suggested change
const handleSubmitPromise = () => {
ruleFormRef.value.validate()
}
const formData = ref({
name: '',
age: ''
const handleSubmitPromise = async () => {
try {
await ruleFormRef.value.validate()
// Handle successful validation
console.log('Form validated successfully:', formData.value)
} catch (error) {
console.error('Validation failed:', error)
}
}
const formData = ref({
name: '',
age: null
})

})
</script>

<style scoped>
.demo-form {
width: 380px;
}
</style>
61 changes: 57 additions & 4 deletions examples/sites/demos/pc/app/config-provider/base.vue
Original file line number Diff line number Diff line change
@@ -1,26 +1,64 @@
<template>
<div>
<tiny-config-provider :design="design">
<tiny-alert type="warning" description="type 为 warning"></tiny-alert>
<div class="demo-form">
<tiny-alert type="warning" description="全局配置组件的默认行为"></tiny-alert>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance the alert description to better document the config-provider capabilities.

The current description "全局配置组件的默认行为" (default behavior of global configuration component) is too generic. Consider expanding it to highlight key features and use cases.

-<tiny-alert type="warning" description="全局配置组件的默认行为"></tiny-alert>
+<tiny-alert type="warning" description="通过 ConfigProvider 组件,你可以全局配置组件的默认属性、样式主题和行为。本示例展示了表单组件的自定义配置。"></tiny-alert>
📝 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.

Suggested change
<tiny-alert type="warning" description="全局配置组件的默认行为"></tiny-alert>
<tiny-alert type="warning" description="通过 ConfigProvider 组件,你可以全局配置组件的默认属性、样式主题和行为。本示例展示了表单组件的自定义配置。"></tiny-alert>

<tiny-form ref="ruleFormRef" :model="formData">
<tiny-form-item label="年龄" prop="age" required>
<tiny-numeric v-model="formData.age"></tiny-numeric>
</tiny-form-item>
<tiny-form-item label="姓名" prop="name" required>
<tiny-input v-model="formData.name"></tiny-input>
</tiny-form-item>
<tiny-form-item>
<tiny-button @click="handleSubmitPromise" type="primary"> 校验 </tiny-button>
</tiny-form-item>
</tiny-form>
</div>
</tiny-config-provider>
</div>
</template>

<script>
import { TinyConfigProvider, TinyAlert, TinyModal } from '@opentiny/vue'
import {
TinyConfigProvider,
TinyAlert,
TinyModal,
TinyForm,
TinyFormItem,
TinyInput,
TinyNumeric,
TinyButton
} from '@opentiny/vue'
import { iconWarningTriangle } from '@opentiny/vue-icon'

export default {
components: {
TinyConfigProvider,
TinyAlert
TinyAlert,
TinyForm,
TinyFormItem,
TinyInput,
TinyNumeric,
TinyButton
},
data() {
return {
design: {
name: 'smb', // 设计规范名称
name: 'x-design', // 设计规范名称
version: '1.0.0', // 设计规范版本号
components: {
Form: {
props: {
hideRequiredAsterisk: true
}
},
Button: {
props: {
resetTime: 0,
round: true
}
},
Comment on lines +48 to +61
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add documentation for configuration options.

The design configuration lacks proper documentation explaining the purpose and impact of each setting. Consider adding detailed comments for better understanding.

 design: {
-  name: 'x-design', // 设计规范名称
+  name: 'x-design', // 设计规范名称,用于指定全局样式主题
   version: '1.0.0', // 设计规范版本号
   components: {
     Form: {
+      // Form 组件的全局配置
       props: {
+        // 隐藏表单项必填字段的星号标记
         hideRequiredAsterisk: true
       }
     },
     Button: {
+      // Button 组件的全局配置
       props: {
+        // 设置按钮点击后的重置时间(毫秒)
         resetTime: 0,
+        // 设置按钮为圆角样式
         round: 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.

Suggested change
name: 'x-design', // 设计规范名称
version: '1.0.0', // 设计规范版本号
components: {
Form: {
props: {
hideRequiredAsterisk: true
}
},
Button: {
props: {
resetTime: 0,
round: true
}
},
name: 'x-design', // 设计规范名称,用于指定全局样式主题
version: '1.0.0', // 设计规范版本号
components: {
Form: {
// Form 组件的全局配置
props: {
// 隐藏表单项必填字段的星号标记
hideRequiredAsterisk: true
}
},
Button: {
// Button 组件的全局配置
props: {
// 设置按钮点击后的重置时间(毫秒)
resetTime: 0,
// 设置按钮为圆角样式
round: true
}
},

Alert: {
icons: {
warning: iconWarningTriangle()
Expand All @@ -44,8 +82,23 @@ export default {
}
}
}
},
formData: {
name: '',
age: ''
}
}
},
methods: {
handleSubmitPromise() {
this.$refs.ruleFormRef.validate()
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve form validation handling and user feedback.

The current validation implementation doesn't handle the validation result or provide user feedback.

 handleSubmitPromise() {
-  this.$refs.ruleFormRef.validate()
+  this.$refs.ruleFormRef.validate((valid) => {
+    if (valid) {
+      TinyModal.message({ type: 'success', message: '验证通过' })
+    } else {
+      TinyModal.message({ type: 'error', message: '请检查表单填写是否正确' })
+    }
+  })
 }
📝 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.

Suggested change
handleSubmitPromise() {
this.$refs.ruleFormRef.validate()
}
handleSubmitPromise() {
this.$refs.ruleFormRef.validate((valid) => {
if (valid) {
TinyModal.message({ type: 'success', message: '验证通过' })
} else {
TinyModal.message({ type: 'error', message: '请检查表单填写是否正确' })
}
})
}

}
}
</script>

<style scoped>
.demo-form {
width: 380px;
}
</style>
18 changes: 17 additions & 1 deletion examples/sites/demos/pc/app/config-provider/basic.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,23 @@ import { test, expect } from '@playwright/test'
test('测试自定义事件', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('config-provider#base')
await page.locator('.tiny-config-provider > .tiny-alert > .tiny-svg').nth(1).click()

// 验证自定义方法
const demo = page.locator('#base')
await demo.locator('.tiny-config-provider .tiny-alert > .tiny-alert__close').click()
await page.waitForTimeout(500)
await expect(page.locator('.tiny-modal > .tiny-modal__box').nth(1)).toHaveText('触发自定方法')

Comment on lines +7 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve test reliability and maintainability

Consider the following improvements:

  1. Replace waitForTimeout with waitForSelector or waitForEvent for more reliable testing
  2. Consider using data-testid attributes instead of class selectors
  3. Consider using English comments for better international collaboration
- // 验证自定义方法
+ // Verify custom method
  const demo = page.locator('#base')
  await demo.locator('.tiny-config-provider .tiny-alert > .tiny-alert__close').click()
- await page.waitForTimeout(500)
+ await page.waitForSelector('.tiny-modal > .tiny-modal__box')
  await expect(page.locator('.tiny-modal > .tiny-modal__box').nth(1)).toHaveText('触发自定方法')

Committable suggestion skipped: line range outside the PR's diff.

// 验证必填星号
await expect(demo.locator('.tiny-form')).toBeVisible()
const beforeElement = await page.evaluate(() => {
const labelBefore = document.querySelector('.tiny-form .tiny-form-item__label')
const { display, content } = window.getComputedStyle(labelBefore, '::before')
return { display, content }
})
expect(beforeElement.content).toBe('none')

// 验证按钮点击禁用时间
await demo.locator('.tiny-button').click()
await expect(demo.locator('.tiny-button')).not.toBeDisabled({ timeout: 300 })
Comment on lines +22 to +24
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve button state test reliability and clarity

The current test uses a fixed timeout and lacks clear documentation of the expected behavior.

- // 验证按钮点击禁用时间
+ // Verify button state after click
  await demo.locator('.tiny-button').click()
- await expect(demo.locator('.tiny-button')).not.toBeDisabled({ timeout: 300 })
+ // Wait for button to become enabled and verify its state
+ await expect(demo.locator('.tiny-button')).toBeEnabled()

Additionally, consider adding a comment explaining the expected behavior and why we're testing this specific interaction.

📝 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.

Suggested change
// 验证按钮点击禁用时间
await demo.locator('.tiny-button').click()
await expect(demo.locator('.tiny-button')).not.toBeDisabled({ timeout: 300 })
// Verify button state after click
await demo.locator('.tiny-button').click()
// Wait for button to become enabled and verify its state
await expect(demo.locator('.tiny-button')).toBeEnabled()

})

This file was deleted.

14 changes: 0 additions & 14 deletions examples/sites/demos/pc/app/config-provider/form.spec.ts

This file was deleted.

51 changes: 0 additions & 51 deletions examples/sites/demos/pc/app/config-provider/form.vue

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ export default {
'en-US': 'Basic Use'
},
desc: {
'zh-CN': '可通过<code>design</code>属性设置自定义不同设计规范的图标和逻辑。',
'zh-CN':
'可通过<code> design </code>属性设置自定义不同设计规范的图标和逻辑,例如:全局配置 Form 表单组件的必填星号是否默认显示、Button 组件的点击后的禁用时间和是否默认圆角。',
'en-US':
'Icons and logic for different design specifications can be customized through the <code>design</code> attribute configuration.'
},
Expand Down Expand Up @@ -40,18 +41,6 @@ export default {
'en-US': 'Container labels can be customized through<code>tag</code>.'
},
codeFiles: ['tag.vue']
},
{
demoId: 'form',
name: {
'zh-CN': '隐藏表单必填星号',
'en-US': 'Hide all form required asterisks'
},
desc: {
'zh-CN': '通过 <code>design</code> 设置所有表单组件默认不显示必填星号。',
'en-US': 'Set the all form component via <code>design</code> to not display required asterisks by default.'
},
codeFiles: ['form.vue']
}
]
}
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/menus.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export const cmpMenus = [
'children': [
{ 'nameCn': '按钮', 'name': 'Button', 'key': 'button' },
{ 'nameCn': '按钮组', 'name': 'ButtonGroup', 'key': 'button-group' },
{ 'nameCn': '全局配置', 'name': 'ConfigProvider', 'key': 'config-provider' },
{ 'nameCn': '容器布局', 'name': 'Container', 'key': 'container' },
{ 'nameCn': '图标', 'name': 'Icon', 'key': 'icon' },
// { 'nameCn': '多色图标', 'name': 'IconMulticolor', 'key': 'icon-multicolor' }, // 隐藏路由,目前只有saas使用
Expand Down Expand Up @@ -309,7 +310,6 @@ export const cmpMenus = [
'key': 'cmp-other-components',
'children': [
{ 'nameCn': '公告牌', 'name': 'BulletinBoard', 'key': 'bulletin-board' },
{ 'nameCn': '全局配置', 'name': 'ConfigProvider', 'key': 'config-provider' },
{ 'nameCn': '图片裁剪', 'name': 'Crop', 'key': 'crop' },
{ 'nameCn': '弹窗选择 ', 'name': 'DialogSelect ', 'key': 'dialog-select' },
{ 'nameCn': '过滤器面板', 'name': 'FilterPanel', 'key': 'filter-panel' },
Expand Down
15 changes: 12 additions & 3 deletions packages/renderless/src/button/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,27 @@ import type { IButtonRenderlessParams, IButtonState } from '@/types'
import { xss } from '../common/xss'

export const handleClick =
({ emit, props, state }: Pick<IButtonRenderlessParams, 'emit' | 'props' | 'state'>) =>
({ emit, props, state, designConfig }: Pick<IButtonRenderlessParams, 'emit' | 'props' | 'state' | 'designConfig'>) =>
(event: MouseEvent): void => {
const urlHref = xss.filterUrl(props.href)
const DEFAULT_RESETTIME = 1000

let resetTime = DEFAULT_RESETTIME

if (props.resetTime !== DEFAULT_RESETTIME) {
resetTime = props.resetTime
} else {
resetTime = designConfig?.props?.resetTime ?? props.resetTime
}
Comment on lines +19 to +27
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Simplify reset time logic

The current implementation can be simplified for better maintainability and readability.

Consider these improvements:

  1. Move the constant to the top level:
+ const DEFAULT_RESETTIME = 1000;

export const handleClick =
  ({ emit, props, state, designConfig }: Pick<IButtonRenderlessParams, 'emit' | 'props' | 'state' | 'designConfig'>) =>
  (event: MouseEvent): void => {
-   const DEFAULT_RESETTIME = 1000
  1. Simplify the reset time logic:
-   let resetTime = DEFAULT_RESETTIME
-
-   if (props.resetTime !== DEFAULT_RESETTIME) {
-     resetTime = props.resetTime
-   } else {
-     resetTime = designConfig?.props?.resetTime ?? props.resetTime
-   }
+   const resetTime = props.resetTime === DEFAULT_RESETTIME
+     ? (designConfig?.props?.resetTime ?? DEFAULT_RESETTIME)
+     : props.resetTime;

Committable suggestion skipped: line range outside the PR's diff.


if (urlHref) {
location.href = urlHref
} else if (props.nativeType === 'button' && props.resetTime > 0) {
} else if (props.nativeType === 'button' && resetTime > 0) {
state.disabled = true

state.timer = window.setTimeout(() => {
state.disabled = false
}, props.resetTime)
}, resetTime)
}

emit('click', event)
Expand Down
Loading
Loading