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(form-item): label-width support auto #2114

Merged
merged 136 commits into from
Jan 6, 2022

Conversation

doom-9-zz
Copy link
Contributor

No description provided.

doom-9 and others added 30 commits June 17, 2021 17:42
@doom-9-zz
Copy link
Contributor Author

close #2087

@lgtm-com
Copy link

lgtm-com bot commented Jan 5, 2022

This pull request introduces 1 alert when merging 507eb3d into 14910a0 - view on LGTM.com

new alerts:

  • 1 for Ineffective parameter type

@codecov
Copy link

codecov bot commented Jan 5, 2022

Codecov Report

Merging #2114 (a296a23) into main (e33eff3) will decrease coverage by 0.02%.
The diff coverage is 34.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2114      +/-   ##
==========================================
- Coverage   64.73%   64.70%   -0.03%     
==========================================
  Files         899      899              
  Lines       17639    17656      +17     
  Branches     4186     4194       +8     
==========================================
+ Hits        11418    11425       +7     
- Misses       5436     5445       +9     
- Partials      785      786       +1     
Impacted Files Coverage Δ
src/form/src/interface.ts 100.00% <ø> (ø)
src/form/src/utils.ts 64.21% <10.00%> (-6.72%) ⬇️
src/form/src/FormItem.tsx 39.72% <50.00%> (+0.99%) ⬆️
src/form/src/Form.tsx 46.66% <100.00%> (+5.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e33eff3...a296a23. Read the comment docs.

@@ -27,7 +27,7 @@ partially-apply-rules
| --- | --- | --- | --- |
| disabled | `boolean` | `false` | Whether to disable the form. |
| inline | `boolean` | `false` | Whether to display as an inline form. |
| label-width | `number \| string` | `undefined` | The width of label. Particularly useful when `label-placement` is set to `'left'`. |
| label-width | `number \| string` | `undefined` | The width of label. Particularly useful when `label-placement` is set to `'left'`,Width `'auto'` is supported. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

number \| number \| 'auto'

'auto' means label width will be auto adjusted

Copy link
Collaborator

Choose a reason for hiding this comment

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

下面的也顺便改改

@@ -64,6 +72,13 @@ export default defineComponent({
useTheme('Form', 'Form', style, formLight, props, mergedClsPrefixRef)
// from path to form-item
const formItems: Record<string, FormItemInst[]> = {}
// label-width = 'auto'
const autoComputedWidth = ref(0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

maxChildLabelWidth

@@ -64,6 +72,13 @@ export default defineComponent({
useTheme('Form', 'Form', style, formLight, props, mergedClsPrefixRef)
// from path to form-item
const formItems: Record<string, FormItemInst[]> = {}
// label-width = 'auto'
const autoComputedWidth = ref(0)
const changeAutoComputedWidth = (currentWidth: number): void => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

deriveMaxChildLabelWidth

provide(formItemLabelWidthInjectionKey, {
autoComputedWidth,
changeAutoComputedWidth
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

跟着 formInjectionKey 就好了

Comment on lines 342 to 347
onMounted(() => {
if (labelElementRef.value !== null) {
NFormItemLabelWidth?.changeAutoComputedWidth(
Number(getComputedStyle(labelElementRef.value).width.slice(0, -2))
)
}
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

理论上最好还是走 ResizeObserver

Copy link
Collaborator

Choose a reason for hiding this comment

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

比如给 label 里面的内容套个 span,然后监听 resize,需要注意 * 也要放在 span 里面,因为 * 占据空间

Comment on lines 24 to 53
const mergedLabelWidthRef = computed(() => {
if (mergedLabelPlacementRef.value === 'top') return
const { labelWidth } = props
const autoComputedWidth = NForm?.maxChildLabelWidth.value

if (labelWidth === 'auto') {
if (autoComputedWidth !== undefined) {
return formatLength(autoComputedWidth)
} else {
return undefined
}
}

if (labelWidth !== undefined) {
return formatLength(labelWidth)
}

if (NForm?.labelWidth !== undefined) {
return formatLength(NForm.labelWidth)
if (NForm?.labelWidth.value === 'auto') {
if (autoComputedWidth !== undefined) {
return formatLength(autoComputedWidth)
} else {
return undefined
}
}

if (NForm?.labelWidth.value !== undefined) {
return formatLength(NForm.labelWidth.value)
}
return undefined
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

这段逻辑写的不干净,改完之后是这样的

  const mergedLabelWidthRef = computed(() => {
    if (mergedLabelPlacementRef.value === 'top') return
    const { labelWidth } = props

    if (labelWidth !== undefined && labelWidth !== 'auto') {
      return formatLength(labelWidth)
    }

    if (labelWidth === 'auto' || NForm?.props.labelWidth === 'auto') {
      const autoComputedWidth = NForm?.maxChildLabelWidthRef.value
      if (autoComputedWidth !== undefined) {
        return formatLength(autoComputedWidth)
      } else {
        return undefined
      }
    }

    if (NForm?.props.labelWidth !== undefined) {
      return formatLength(NForm.props.labelWidth)
    }
    return undefined
  })

Copy link
Collaborator

Choose a reason for hiding this comment

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

原本 auto 的 case 是重复的

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants