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

feature/form: add show-label prop #865

Merged
merged 15 commits into from
Aug 19, 2021
Merged

Conversation

kev1nzh37
Copy link
Collaborator

@vercel
Copy link

vercel bot commented Aug 13, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tusimple/naive-ui/6AfzVYc4G5WtfkV3i7zYTaCMAgGp
✅ Preview: https://naive-ui-git-fork-kev1nzh37-feature-form-tusimple.vercel.app

@codecov
Copy link

codecov bot commented Aug 13, 2021

Codecov Report

Merging #865 (31591c3) into main (19282b6) will increase coverage by 0.03%.
The diff coverage is 85.71%.

❗ Current head 31591c3 differs from pull request most recent head 6277b82. Consider uploading reports for the commit 6277b82 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #865      +/-   ##
==========================================
+ Coverage   44.38%   44.41%   +0.03%     
==========================================
  Files         509      509              
  Lines       12449    12454       +5     
  Branches     3498     3500       +2     
==========================================
+ Hits         5526     5532       +6     
  Misses       5914     5914              
+ Partials     1009     1008       -1     
Impacted Files Coverage Δ
src/form/src/Form.tsx 29.41% <ø> (ø)
src/form/src/FormItem.tsx 29.13% <50.00%> (+0.78%) ⬆️
src/form/src/utils.ts 66.66% <100.00%> (+2.38%) ⬆️

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 19282b6...6277b82. Read the comment docs.

Copy link
Collaborator

@Volankey Volankey left a comment

Choose a reason for hiding this comment

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

实现参考一下 showRequireMark,form及form-item都应该有

CHANGELOG.en-US.md Outdated Show resolved Hide resolved
CHANGELOG.en-US.md Outdated Show resolved Hide resolved
CHANGELOG.zh-CN.md Outdated Show resolved Hide resolved
@kev1nzh37
Copy link
Collaborator Author

实现参考一下 showRequireMark,form及form-item都应该有

好的,明天我加一下

Co-authored-by: Yugang Cao <[email protected]>
Co-authored-by: Yugang Cao <[email protected]>
@XieZongChen
Copy link
Collaborator

XieZongChen commented Aug 13, 2021

尝试给当前pr加一下测试吧

@XieZongChen XieZongChen linked an issue Aug 13, 2021 that may be closed by this pull request
@07akioni
Copy link
Collaborator

加一个 Demo,然后按照我的理解,不展示 label 的情况下应该也没有 label 占位,所以样式也需要修改。

@kev1nzh37
Copy link
Collaborator Author

收到。

src/form/src/FormItem.tsx Outdated Show resolved Hide resolved
@kev1nzh37
Copy link
Collaborator Author

似乎逻辑上有点问题,我明天继续看下。

@kev1nzh37
Copy link
Collaborator Author

逻辑似乎不能融合进 mergedShowLabelRef, 因为 no-label 这个 class 和 label的内容没关系。把代码还原回来了。

@lgtm-com
Copy link

lgtm-com bot commented Aug 18, 2021

This pull request introduces 1 alert when merging 08f85c8 into 6530474 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@07akioni
Copy link
Collaborator

LGTM

Comment on lines 27 to 29

- `n-message-provider` add `container-style` prop
- `n-form` add `show-label` prop, closes [#858](https://github.com/TuSimple/naive-ui/issues/858).
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个的位置得改一下,挪到没发布的版本

Comment on lines 26 to 28

- `n-form` 新增 `show-label` 属性,关闭 [#858 ](https://github.com/TuSimple/naive-ui/issues/858)
- `n-message-provider` 新增 `container-style` 属性
Copy link
Collaborator

Choose a reason for hiding this comment

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

挪一下位置

src/form/demos/enUS/index.demo-entry.md Show resolved Hide resolved
src/form/demos/enUS/index.demo-entry.md Show resolved Hide resolved
Copy link
Collaborator

@07akioni 07akioni left a comment

Choose a reason for hiding this comment

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

image

这个是期望的效果。

我研究了一下,好像 label = false 就能关掉 label,这个是目前已经实现的,通过 n-form-item--no-label 这个类。我们未来只通过 show-label 来控制这个行为,然后不允许 label 传 bool了。剩下的和现在一致。

@kev1nzh37
Copy link
Collaborator Author

image

这个是期望的效果。

我研究了一下,好像 label = false 就能关掉 label,这个是目前已经实现的,通过 n-form-item--no-label 这个类。我们未来只通过 show-label 来控制这个行为,然后不允许 label 传 bool了。剩下的和现在一致。

了解了,白天提交。

CHANGELOG.en-US.md Outdated Show resolved Hide resolved
CHANGELOG.zh-CN.md Outdated Show resolved Hide resolved
rhengles pushed a commit to arijs/naive-ui that referenced this pull request Oct 20, 2021
* feat(form): add show-label prop and test (tusen-ai#858)

* docs(form): add show-label doc and fix form item label desc (tusen-ai#858)

* Update CHANGELOG.en-US.md

Co-authored-by: Yugang Cao <[email protected]>

* Update CHANGELOG.zh-CN.md

Co-authored-by: Yugang Cao <[email protected]>

* feat(form): rewrite show-label and add tests (tusen-ai#858)

* docs(form): add show-label demo (tusen-ai#858)

* feat(form): change show-label computed  (tusen-ai#858)

* feat(form): fix show-label (tusen-ai#858)

* feat(form): fix bug (tusen-ai#858)

* feat(form): update show-label scripts (tusen-ai#858)

* docs(form): update show-label order and label description (tusen-ai#858)

* Apply suggestions from code review

Co-authored-by: kev1nzh <[email protected]>
Co-authored-by: Yugang Cao <[email protected]>
Co-authored-by: kev1nzh_ark <[email protected]>
Co-authored-by: 07akioni <[email protected]>
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.

Form hide label props 隱藏表單label
5 participants