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.

Sign up for GitHub

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(form): [form] form component adapt to old theme #2562

Merged
merged 1 commit into from
Nov 28, 2024
Merged

feat(form): [form] form component adapt to old theme #2562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion examples/sites/demos/apis/form.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export default {
{
name: 'hide-required-asterisk',
type: 'boolean',
defaultValue: 'true',
defaultValue: 'false',
desc: {
'zh-CN': '是否隐藏必填字段的标签旁边的红色星号',
'en-US': 'Whether to hide the red asterisk next to the label of mandatory fields'
Expand Down
40 changes: 40 additions & 0 deletions examples/sites/demos/pc/app/config-provider/form-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<template>
<div>
<tiny-config-provider :design="design">
<tiny-form :model="formData" class="demo-form">
<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>
</tiny-config-provider>
</div>
</template>

<script setup>
import { ref } from 'vue'
import { TinyConfigProvider, TinyForm, TinyFormItem, TinyInput, TinyNumeric } from '@opentiny/vue'

const design = ref({
name: 'smb', // 设计规范名称
version: '1.0.0', // 设计规范版本号
components: {
Form: {
hideRequiredAsterisk: true
}
}
})

const formData = ref({
name: '',
age: ''
})
</script>

<style scoped>
.demo-form {
width: 380px;
}
</style>
14 changes: 14 additions & 0 deletions examples/sites/demos/pc/app/config-provider/form.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { test, expect } from '@playwright/test'

test('测试隐藏星号', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('config-provider#form')

await expect(page.locator('#form .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')
Comment on lines +8 to +13
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 style evaluation reliability and clarity

The current style evaluation could be more robust and self-documenting:

  1. Add error handling for missing elements
  2. Make assertions more specific
  3. Add comments explaining the expected behavior
   const beforeElement = await page.evaluate(() => {
-    const labelBefore = document.querySelector('.tiny-form .tiny-form-item__label')
+    const labelBefore = document.querySelector('.tiny-form .tiny-form-item__label');
+    if (!labelBefore) {
+      throw new Error('Form label element not found');
+    }
     const { display, content } = window.getComputedStyle(labelBefore, '::before')
     return { display, content }
   })
-  expect(beforeElement.content).toBe('none')
+  // Verify that the required asterisk is hidden
+  expect(beforeElement.content, 'Required asterisk should be hidden').toBe('none')
📝 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 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')
const beforeElement = await page.evaluate(() => {
const labelBefore = document.querySelector('.tiny-form .tiny-form-item__label');
if (!labelBefore) {
throw new Error('Form label element not found');
}
const { display, content } = window.getComputedStyle(labelBefore, '::before')
return { display, content }
})
// Verify that the required asterisk is hidden
expect(beforeElement.content, 'Required asterisk should be hidden').toBe('none')

})
51 changes: 51 additions & 0 deletions examples/sites/demos/pc/app/config-provider/form.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<template>
<div>
<tiny-config-provider :design="design">
<tiny-form :model="formData" class="demo-form">
<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>
</tiny-config-provider>
</div>
</template>

<script>
import { TinyConfigProvider, TinyForm, TinyFormItem, TinyInput, TinyNumeric } from '@opentiny/vue'

export default {
components: {
TinyConfigProvider,
TinyForm,
TinyFormItem,
TinyInput,
TinyNumeric
},
data() {
return {
design: {
name: 'smb', // 设计规范名称
version: '1.0.0', // 设计规范版本号
components: {
Form: {
hideRequiredAsterisk: true
}
}
},
formData: {
name: '',
age: ''
}
}
}
}
</script>

<style scoped>
.demo-form {
width: 380px;
}
</style>
12 changes: 12 additions & 0 deletions examples/sites/demos/pc/app/config-provider/webdoc/config-provider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ 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']
}
]
}
0 ...app/form/smb-required-composition-api.vue → ...pp/form/hide-required-composition-api.vue
View file
Open in desktop
File renamed without changes.
4 changes: 2 additions & 2 deletions ...es/demos/pc/app/form/smb-required.spec.ts → ...s/demos/pc/app/form/hide-required.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { test, expect } from '@playwright/test'

test('必填项红色星号', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('form#smb-required')
await page.goto('form#hide-required')

const demo = page.locator('#smb-required')
const demo = page.locator('#hide-required')
Comment on lines +5 to +7
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistency found in demo ID configuration

The test file has been updated to use 'hide-required', but there's an inconsistency in the configuration:

  • examples/sites/demos/apis/form.js still references smb-required
  • The demo ID 'hide-required' is properly configured in examples/sites/demos/pc/app/form/webdoc/form.js

This mismatch needs to be fixed to maintain consistency across the codebase.

🔗 Analysis chain

LGTM! Verify the demo ID changes across related files.

The navigation and selector updates are consistent with the renamed demo ID.

Let's verify the demo ID changes are consistent across related files:

The previous results show that 'smb-required' is still present in apis/form.js while the test was updated to use 'hide-required'. Let's verify all occurrences and related files to ensure consistency.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all references to the old demo ID 'smb-required' have been updated to 'hide-required'

# Search for any remaining references to the old ID
echo "Checking for any remaining references to 'smb-required'..."
rg "smb-required"

# Search for the new ID to confirm it exists in demo configuration
echo "Verifying 'hide-required' exists in demo files..."
rg "hide-required" "examples/sites/demos/webdoc/form.js"

Length of output: 464

Script:

#!/bin/bash
# Let's check all possible demo configuration files and related files
echo "Checking for 'hide-required' in all files..."
rg "hide-required"

echo -e "\nChecking form demo configuration files..."
fd "form.js" examples/sites/demos/

Length of output: 2455

const form = demo.locator('.tiny-form')
const firstLabel = form.locator('.tiny-form-item__label').first()
const switchBtn = demo.locator('.tiny-switch')
Expand Down
0 .../sites/demos/pc/app/form/smb-required.vue → ...sites/demos/pc/app/form/hide-required.vue
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/form/slot-label-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</tiny-form-item>
<tiny-form-item prop="url">
<template #label>
<div class="custom-label" v-auto-tip>超过两行文字，省略显示</div>
<div class="custom-label" v-auto-tip>超过两行文字，显示省略号</div>
</template>
<tiny-input v-model="createData.url"></tiny-input>
</tiny-form-item>
Expand Down
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/form/slot-label.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ test('测试表单标签文本插槽', async ({ page }) => {

await expect(formItem.first().locator('.tiny-form-item__label')).toHaveText('必填')
await expect(formItem.nth(1).locator('.tiny-form-item__label')).toHaveText('日期')
await expect(formItem.nth(2).locator('.tiny-form-item__label')).toHaveText('超过两行文字，省略显示')
await expect(formItem.nth(2).locator('.tiny-form-item__label')).toHaveText('超过两行文字，显示省略号')
await expect(formItem.nth(3).locator('.tiny-form-item__label')).toHaveText('等级')
})
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/form/slot-label.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</tiny-form-item>
<tiny-form-item prop="url">
<template #label>
<div class="custom-label" v-auto-tip>超过两行文字，省略显示</div>
<div class="custom-label" v-auto-tip>超过两行文字，显示省略号</div>
</template>
<tiny-input v-model="createData.url"></tiny-input>
</tiny-form-item>
Expand Down
8 changes: 4 additions & 4 deletions examples/sites/demos/pc/app/form/webdoc/form.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,18 +263,18 @@ export default {
codeFiles: ['group-form.vue']
},
{
demoId: 'smb-required',
demoId: 'hide-required',
name: {
'zh-CN': '必填项红色星号',
'en-US': 'Required items with a red asterisk'
},
desc: {
'zh-CN':
'<p>通过 <code>hide-required-asterisk</code> 设置是否隐藏标签前的红色星号，默认为 <code>true</code> 。</p>',
'<p>通过 <code>hide-required-asterisk</code> 设置是否隐藏标签前的红色星号，默认为 <code>false</code> 。</p>',
'en-US':
'By setting whether to hide the red asterisk in front of the label through <code>hide required asterisk</code> , it defaults to <code>true</code>.'
'By setting whether to hide the red asterisk in front of the label through <code>hide required asterisk</code> , it defaults to <code>false</code>.'
},
codeFiles: ['smb-required.vue']
codeFiles: ['hide-required.vue']
},
{
demoId: 'popper-options',
Expand Down
1 change: 0 additions & 1 deletion packages/design/aurora/src/form/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@ export default {
labelWidth: '100px',
tooltipType: 'error'
},
hideRequiredAsterisk: false,
messageType: 'absolute'
}
1 change: 0 additions & 1 deletion packages/design/saas/src/form/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@ export default {
labelWidth: '100px',
tooltipType: 'error'
},
hideRequiredAsterisk: false,
messageType: 'absolute'
}
2 changes: 1 addition & 1 deletion packages/renderless/src/form/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const computedAutoLabelWidth =
export const computedHideRequiredAsterisk =
({ props, designConfig }: Pick<IFormRenderlessParams, 'props' | 'designConfig'>) =>
(): boolean => {
return props.hideRequiredAsterisk ?? designConfig?.hideRequiredAsterisk ?? true
return props.hideRequiredAsterisk ?? designConfig?.hideRequiredAsterisk ?? false
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Breaking Change: Default behavior change requires attention

The demo files confirm this is a breaking change. Both the Options API and Composition API examples explicitly set hideRequiredAsterisk: true as their default state, indicating this was the expected behavior. Changing the default from true to false will affect any forms that rely on the default behavior.

Action items:

  1. This change needs to be documented in the changelog as a breaking change
  2. Consider implementing one of these solutions:
    • Add a version-specific fallback
    • Create a migration guide
    • Revert to the original default and implement the new behavior behind a feature flag
🔗 Analysis chain

Breaking Change: Required asterisk visibility default behavior has changed

The default behavior for hiding required asterisks has been inverted from true to false. This means required fields will now show asterisks by default, which could affect the appearance of existing forms.

Let's verify the impact on existing forms and documentation:

Action items:

  1. Consider adding a migration guide for this breaking change
  2. Update component documentation to reflect the new default behavior
  3. Consider adding a feature flag or version-specific fallback for backward compatibility

The previous results show extensive usage of hideRequiredAsterisk across the codebase, including demo files. Let's verify the actual implementation and documented behavior to confirm if this is truly a breaking change and understand its impact.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for form usages and documentation that might need updates

# Check for form component usage in templates
rg -t vue "v-tiny-form|tiny-form" -A 5

# Check for hideRequiredAsterisk property usage
rg "hideRequiredAsterisk"

# Check documentation files
fd -e md -e mdx | xargs rg "hideRequiredAsterisk|required asterisk"

Length of output: 3520

Script:

#!/bin/bash
# Let's check the git history to understand if this was indeed a breaking change
git log -L :computedHideRequiredAsterisk:packages/renderless/src/form/index.ts

# Check for any tests related to this functionality
rg -l "hideRequiredAsterisk.*test" 

# Check the demo files to understand the documented behavior
cat examples/sites/demos/pc/app/form/hide-required.vue
cat examples/sites/demos/pc/app/form/hide-required-composition-api.vue

Length of output: 3597

}

export const computedValidateIcon =
Expand Down
Loading