Input: Clear icon doesn't disappear if value is null (vs undefined or empy string) #14733
Conversation
Codecov Report
@@ Coverage Diff @@
## master #14733 +/- ##
=======================================
Coverage 82.35% 82.35%
=======================================
Files 13 13
Lines 17 17
Branches 3 3
=======================================
Hits 14 14
Misses 3 3Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #14733 +/- ##
===========================================
+ Coverage 82.35% 92.73% +10.38%
===========================================
Files 13 236 +223
Lines 17 6222 +6205
Branches 3 1854 +1851
===========================================
+ Hits 14 5770 +5756
- Misses 3 449 +446
- Partials 0 3 +3
Continue to review full report at Codecov.
|
|
Deploy preview for ant-design ready! Built with commit 90b0ad0 |
|
Cloud you add a test case? |
![accesslint[bot]](https://avatars.githubusercontent.com/in/1502?s=60&v=4){kind=small}
| defaultValue={null} | ||
| disabled={false} | ||
| type="text" | ||
| > |
There was a problem hiding this comment.
Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.
| @@ -134,6 +136,26 @@ describe('Input allowClear', () => { | |||
| expect(wrapper.find('input').getDOMNode().value).toEqual(''); | |||
| }); | |||
|
|
|||
| it('should not show icon if value is undefined, null or empty string', () => { | |||
| const wrappers = [null, undefined, ''].map(val => mount(<Input allowClear value={val} />)); | |||
There was a problem hiding this comment.
Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.
| style={null} | ||
| type="text" | ||
| value="" | ||
| /> |
There was a problem hiding this comment.
Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.
|
|
||
| it('should not show icon if defaultValue is undefined, null or empty string', () => { | ||
| const wrappers = [null, undefined, ''].map(val => | ||
| mount(<Input allowClear defaultValue={val} />), |
There was a problem hiding this comment.
Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.
| allowClear={true} | ||
| disabled={false} | ||
| type="text" | ||
| > |
There was a problem hiding this comment.
Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.
| style={null} | ||
| type="text" | ||
| value="" | ||
| /> |
There was a problem hiding this comment.
Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.
| allowClear={true} | ||
| disabled={false} | ||
| type="text" | ||
| > |
There was a problem hiding this comment.
Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.
| style={null} | ||
| type="text" | ||
| value="" | ||
| /> |
There was a problem hiding this comment.
Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.
| disabled={false} | ||
| type="text" | ||
| value="" | ||
| > |
There was a problem hiding this comment.
Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.
| style={null} | ||
| type="text" | ||
| value="" | ||
| /> |
There was a problem hiding this comment.
Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.
|
Thanks for your review. As requested by @afc163 I added a test each for value and defaultValue. Can this be merged? |
|
@zombieJ Something wrong of ci in master. |
Seems react@16.8.0 released: facebook/react#14764 |
This is a ...
What's the background?
If the value becomes null in the Input component, the clear icon is still shown if activated (setting allowClear).
See example: https://codesandbox.io/s/7jp07nyp9x
A check for null needed to be introduced in Input.tsx.
Self Check before Merge