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

refactor(form): [form] refactor form style #2196

Merged
merged 8 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
15 changes: 7 additions & 8 deletions examples/sites/demos/apis/form.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,10 @@ export default {
{
name: 'hide-required-asterisk',
type: 'boolean',
defaultValue: 'false',
defaultValue: 'true',
desc: {
'zh-CN': '是否隐藏必填字段的标签旁边的红色星号,SMB 主题下默认值为true',
'en-US':
'Whether to hide the red asterisk next to the label of mandatory fields, The default value for SMB theme is true'
'zh-CN': '是否隐藏必填字段的标签旁边的红色星号',
'en-US': 'Whether to hide the red asterisk next to the label of mandatory fields'
Comment on lines +47 to +50
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Significant change in default behavior for hide-required-asterisk

The default value for hide-required-asterisk has been changed from false to true. This means that by default, the red asterisk next to mandatory fields will now be hidden unless explicitly set to false.

The description update improves clarity and internationalization.

Please ensure that this change in default behavior is intentional and documented in the changelog, as it may affect existing implementations.

The updated description is clear and concise.

},
mode: ['pc', 'mobile-first'],
pcDemo: 'smb-required',
Expand Down Expand Up @@ -120,7 +119,7 @@ export default {
{
name: 'label-width',
type: 'string',
defaultValue: "'80px'",
defaultValue: "'84px'",
desc: {
'zh-CN': '表单中标签占位宽度',
'en-US': 'Label placeholder width in the form'
Expand All @@ -132,11 +131,11 @@ export default {
},
{
name: 'message-type',
type: "'inline' | 'block'",
defaultValue: '',
type: "'inline' | 'block' | 'absolute'",
defaultValue: "'block'",
desc: {
'zh-CN':
'当 validate-type 设置为 text 时,配置文本类型错误类型,可配置行内或者块级,不设置则为 absolute 定位',
'当 validate-type 设置为 text 时,配置文本类型错误类型,可配置行内或者块级,其他值都为 absolute 定位',
'en-US':
'Configure the text type error type, which can be configured at the inline or block level when validate-type is set to text. The default is absolute positioning'
},
Expand Down
7 changes: 6 additions & 1 deletion packages/design/aurora/src/form/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
export default {
icons: {
validateIcon: ''
},
state: {
labelWidth: '100px',
tooltipType: 'error'
}
},
hideRequiredAsterisk: false,
messageType: ''
}
7 changes: 6 additions & 1 deletion packages/design/saas/src/form/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
export default {
icons: {
validateIcon: ''
},
state: {
labelWidth: '100px',
tooltipType: 'error'
}
},
hideRequiredAsterisk: false,
messageType: ''
}
2 changes: 0 additions & 2 deletions packages/design/smb/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import Popconfirm from './src/popconfirm'
import Drawer from './src/drawer'
import Dropdown from './src/dropdown'
import DropdownItem from './src/dropdown-item'
import Form from './src/form'
import FilterBox from './src/filter-box'
import Grid from './src/grid'
import Guide from './src/guide'
Expand All @@ -30,7 +29,6 @@ export default {
Drawer,
Dropdown,
DropdownItem,
Form,
FilterBox,
Grid,
Guide,
Expand Down
9 changes: 0 additions & 9 deletions packages/design/smb/src/form/index.ts

This file was deleted.

11 changes: 7 additions & 4 deletions packages/renderless/src/form/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ export const computedAutoLabelWidth =
export const computedHideRequiredAsterisk =
({ props, designConfig }: Pick<IFormRenderlessParams, 'props' | 'designConfig'>) =>
(): boolean => {
return props.hideRequiredAsterisk ?? designConfig?.hideRequiredAsterisk ?? false
return props.hideRequiredAsterisk ?? designConfig?.hideRequiredAsterisk ?? true
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Impact of Changing Default Behavior for Hiding Required Asterisks

Changing the default value of hideRequiredAsterisk from false to true affects multiple components across different packages. This alteration will hide required field indicators by default unless explicitly overridden. Please review the following files to ensure this change aligns with the intended design and does not adversely affect form usability:

  • packages/vue/src/form/src/index.ts
  • packages/renderless/types/form-item.type.ts
  • packages/renderless/types/form.type.ts
  • packages/renderless/src/form/vue.ts
  • packages/renderless/src/form-item/vue.ts
  • packages/renderless/src/form/index.ts
  • packages/design/saas/src/form/index.ts
  • packages/design/aurora/src/form/index.ts
🔗 Analysis chain

Verify the impact of changing the default behavior for hiding required asterisks.

The default behavior for hideRequiredAsterisk has been changed from false to true. This means that required field indicators will be hidden by default unless explicitly set. Please ensure this aligns with the intended design and doesn't negatively impact form usability.

To check the impact, run the following script:



🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usages of hideRequiredAsterisk prop
rg --type vue 'hideRequiredAsterisk'

Length of output: 71


Script:

#!/bin/bash
# Search for usages of hideRequiredAsterisk prop in TypeScript files
rg --type typescript 'hideRequiredAsterisk'

Length of output: 78


Script:

#!/bin/bash
# Search for usages of hideRequiredAsterisk prop in TypeScript files
rg 'hideRequiredAsterisk' --glob '*.ts' --glob '*.tsx'

Length of output: 770

}

export const computedValidateIcon =
({ props, designConfig }: Pick<IFormRenderlessParams, 'props' | 'designConfig'>) =>
(): object | null => {
return props.validateIcon ?? designConfig?.icons?.validateIcon ?? null
return props.validateIcon ?? designConfig?.icons?.validateIcon ?? 'icon-error'
}

export const computedIsErrorInline =
Expand All @@ -65,7 +65,7 @@ export const computedIsErrorInline =
if (typeof props.inlineMessage === 'boolean') {
return props.inlineMessage
}
return designConfig?.messageType === 'inline' || false
return designConfig?.messageType === 'inline'
}

export const computedIsErrorBlock =
Expand All @@ -74,7 +74,10 @@ export const computedIsErrorBlock =
if (props.messageType) {
return props.messageType === 'block'
}
return designConfig?.messageType === 'block' || false
if (designConfig && Object.hasOwnProperty.call(designConfig, 'messageType')) {
return designConfig.messageType === 'block'
}
return true
}

export const created =
Expand Down
2 changes: 1 addition & 1 deletion packages/renderless/src/form/vue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export const renderless = (
return false
}
}),
labelWidth: computed(() => props.labelWidth || designConfig?.state?.labelWidth || '80px'),
labelWidth: computed(() => props.labelWidth || designConfig?.state?.labelWidth || '84px'),
tooltipType: computed(() => designConfig?.state?.tooltipType || 'normal')
})

Expand Down
Loading
Loading