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

Jump to bottom

fix(InputNumber): Fix incorrect disable state style for input box com… #42974

Merged
merged 23 commits into from
Jun 16, 2023
Merged

fix(InputNumber): Fix incorrect disable state style for input box com… #42974

merged 23 commits into from
Jun 16, 2023

Conversation

kampiu
Copy link
Contributor

@kampiu kampiu commented Jun 12, 2023 •
edited by afc163
Loading

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Other (about what?)

🔗 Related issue link

  1. fix InputNumber与Input disable的样式不统一 #42878

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English Fix Input and InputNumber disabled style with addons.
🇨🇳 Chinese 修复 Input 和 InputNumber 禁用状态样式。

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

🚀 Summary

🤖 Generated by Copilot at 2bb36f1

This pull request enhances the InputNumber component by fixing a bug with the controls prop and improving the disabled styles. It modifies components/input-number/index.tsx and components/input-number/style/index.ts.

🔍 Walkthrough

🤖 Generated by Copilot at 2bb36f1

  • Fix bug where controls prop of InputNumber component was not converted to boolean, causing up and down buttons to disappear when controls was false (link)
  • Add new CSS class name to wrapper element of InputNumber component when disabled prop is true, and apply disabled styles to wrapper and its children, such as cursor and background color (link, link, link)
  • Remove redundant CSS rules for disabled state of InputNumber component that were already applied to input element (link)

@kampiu
fix(InputNumber): Fix incorrect disable state style for input box com…
2bb36f1 
…ponents with front and rear label configuration
@github-actions
Copy link
Contributor

github-actions bot commented Jun 12, 2023 •
edited
Loading

@kampiu
Copy link
Contributor Author

kampiu commented Jun 12, 2023

prefix与addonAfter / addonBefore的配置效果如下:
before:
image
after:
image

@kampiu
Copy link
Contributor Author

kampiu commented Jun 12, 2023

InputNumber的props中，prefix与addonAfter/addonBefore是分别用不同的className包裹，只有在prefix生效时才会有${prefixCls}-affix-wrapper-disabled。所以我打算将 ${prefixCls}-affix-wrapper-disabled的类名往上提一层，放置在判断prefix与addonAfter/addonBefore之外。

@afc163
Copy link
Member

afc163 commented Jun 12, 2023

#42971 这样是否就足够了

@kampiu
Copy link
Contributor Author

kampiu commented Jun 12, 2023 •
edited
Loading

我认为不太足够，我还原了他的PR代码，预览到的效果如下：
image

从图中的结果我发现 addonAfter、addonABefore 的禁用状态下字体颜色不正确。

并且我认为${prefixCls}-affix-wrapper-disabled类名在prefix与addonAfter/addonBefore配置都生效的情况下，出现了两次，有两方面看这个问题：

  • 从类名意义理解来说，外层的${prefixCls}-affix-wrapper-disabled更名为${prefixCls}-group-wrapper-disabled更合理。因为同层级类名应命名空间一致（${prefixCls}-group-wrapper${ 状态 }）
  • 另一个方面就是两个类名重复容易让开发者迭代中出现样式效果重叠问题，好比如其类名下 > div${componentCls}这个样式会导致多个匹配选择器的块被修改样式，并且当样式为background: rgba(0, 0, 0, 0.02)之类时出现背景颜色在块重迭处加深问题。

综上所述，这些问题我在我的PR中也是有考虑到并且都已经处理了。

@MadCcc
Copy link
Member

MadCcc commented Jun 13, 2023

来个测试用例吧

@afc163
Copy link
Member

afc163 commented Jun 13, 2023 •
edited
Loading

图片

这里还是很奇怪，prefix 这里不应该有色差。

AAA 圆角左侧外围也有多余的 background。

@kampiu
Copy link
Contributor Author

kampiu commented Jun 13, 2023

图片 这里还是很奇怪，prefix 这里不应该有色差。

AAA 圆角左侧外围也有多余的 background。

是的，所以我的PR在style里面做了微小的调整来避免这些问题。

@afc163
Copy link
Member

afc163 commented Jun 13, 2023

放一个 debug demo 来预览一下效果吧。

@kampiu
Copy link
Contributor Author

kampiu commented Jun 13, 2023

prefix与addonAfter / addonBefore的配置效果如下: before: image after: image

效果在这里的 @afc163

@afc163
Copy link
Member

afc163 commented Jun 13, 2023

我想在 https://preview-42974-ant-design.surge.sh/ 这里直接看效果。

@afc163
Copy link
Member

afc163 commented Jun 13, 2023

是的，所以我的PR在style里面做了微小的调整来避免这些问题。

看你截图问题还在

kampiu added 2 commits June 13, 2023 15:30
@kampiu
fix(InputNumber): Add case previews for pre and post tags and prefix …
5dbe672 
…activation in the disabled state in InputNumber, and add corresponding test cases
@kampiu
Merge branch 'ant-design:master' into master
6e41329
@kampiu
Copy link
Contributor Author

kampiu commented Jun 13, 2023

是的，所以我的PR在style里面做了微小的调整来避免这些问题。

看你截图问题还在

我新增了对应prefix与addonAfter/addonBefore的案例 https://preview-42974-ant-design.surge.sh/components/input-number-cn#components-input-number-demo-addon

@afc163
Copy link
Member

afc163 commented Jun 13, 2023

图片

这里还是有色差的。

@kampiu
Copy link
Contributor Author

kampiu commented Jun 13, 2023

这里的色差是原本prefix的色差，就是可以看看这个案例 前缀配置 如果这个被认为是一个问题，我可以连同这个也一起修复了🔧

@afc163
Copy link
Member

afc163 commented Jun 13, 2023

是个问题，这应该是个 bug。

@kampiu
Copy link
Contributor Author

kampiu commented Jun 13, 2023

是个问题，这应该是个 bug。

我更新了，这是下面最新的预览效果。你也可以直接访问这里查看效果 addon禁用下案例
image

@kampiu
Copy link
Contributor Author

kampiu commented Jun 13, 2023

@afc163 我合并处理了你所指出的地方，并且移除了无效/重复的not-allowed使用。🔧

@afc163
Copy link
Member

afc163 commented Jun 14, 2023

snapshot 更新一下。

@afc163
Copy link
Member

afc163 commented Jun 14, 2023

snapshot 还是有问题。

@afc163
Copy link
Member

afc163 commented Jun 15, 2023

并没有修复，依然报错

图片

@kampiu
Copy link
Contributor Author

kampiu commented Jun 15, 2023

我看测试的快照好像又不通过😭，我看看啥回事

@codecov
Copy link

codecov bot commented Jun 16, 2023 •
edited
Loading

Codecov Report

Patch and project coverage have no change.

Comparison is base (167b2bd) 100.00% compared to head (a2b5475) 100.00%.

Additional details and impacted files 
@@            Coverage Diff            @@
##            master    #42974   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          648       648           
  Lines        10957     10957           
  Branches      2975      2975           
=========================================
  Hits         10957     10957
Impacted Files Coverage Δ
components/input-number/index.tsx 100.00% <ø> (ø)
components/input-number/style/index.ts 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@afc163
Copy link
Member

afc163 commented Jun 16, 2023

终于好了👍🏻

@afc163 afc163 merged commit 0dcd6e8 into ant-design:master Jun 16, 2023
@kampiu
Copy link
Contributor Author

kampiu commented Jun 16, 2023

终于好了👍🏻

非常感谢你帮我完成一次PR的体验，这个过程中我看了ant-design里相关的github工作流以及实践了ant-design的PR规范，学到了不少

@afc163
Copy link
Member

afc163 commented Jun 16, 2023

恭喜成为 antd 贡献者~

@zombieJ zombieJ mentioned this pull request Jun 19, 2023
Merged
16 tasks
BoyYangzai pushed a commit to BoyYangzai/ant-design that referenced this pull request Jun 19, 2023
@kampiu
fix(InputNumber): Fix incorrect disable state style for input box com… (
39d8093 
ant-design#42974)

* fix(InputNumber): Fix incorrect disable state style for input box components with front and rear label configuration

* fix(InputNumber): Add case previews for pre and post tags and prefix activation in the disabled state in InputNumber, and add corresponding test cases

* fix(InputNumber): Fix the issue of incorrect inputNumber background color caused by the original prefix icon being disabled

* fix(InputNumber): Remove the toggle disable state interaction in the addon case

* fix(InputNumber): Optimize and merge style processing

* fix(InputNumber): Adjust the addon case and remove irrelevant cases

* fix(InputNumber): Update the snapshot in the InputNumber component unit test

* fix(InputNumber): Fix the issue of incorrect test snapshots caused by adjusting the controls in the disabled state display

* fix(InputNumber): Fix the issue of incorrect test snapshots caused by adjusting the controls in the disabled state display

* fix(InputNumber): Fix the issue of incorrect test snapshots caused by adjusting the controls in the disabled state display

* fix(InputNumber): Fix the issue of incorrect test snapshots caused by adjusting the controls in the disabled state display

* fix(InputNumber): Fix the issue of incorrect test snapshots caused by adjusting the controls in the disabled state display
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MadCcc MadCcc MadCcc left review comments

@afc163 afc163 afc163 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

InputNumber与Input disable的样式不统一
3 participants
@kampiu @afc163 @MadCcc