-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
fix(module:date-picker): open on enter and focus on inner input #3804
fix(module:date-picker): open on enter and focus on inner input #3804
Conversation
Deploy preview for ng-zorro-master ready! Built with commit 4db785f |
@@ -135,6 +139,7 @@ export class NzPickerComponent implements OnInit, AfterViewInit { | |||
if (this.realOpenState) { | |||
this.overlayOpen = false; | |||
this.openChange.emit(this.overlayOpen); | |||
this.focus(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should need a test for focus
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added 'should focus on the trigger after a click outside' tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -33,6 +33,7 @@ | |||
[locale]="locale" | |||
[disabledDate]="disabledDate" | |||
[format]="format" | |||
[autoFocus]="partType != 'right'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[autoFocus]="partType != 'right'" | |
[autoFocus]="partType !== 'right'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Thank you @jimmytheneutrino, this will close #3530. |
Codecov Report
@@ Coverage Diff @@
## master #3804 +/- ##
==========================================
+ Coverage 95.39% 95.39% +<.01%
==========================================
Files 712 712
Lines 14633 14640 +7
Branches 1929 1930 +1
==========================================
+ Hits 13959 13966 +7
Misses 244 244
Partials 430 430
Continue to review full report at Codecov.
|
3b0ee51
to
fa40ad5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two problems:
- If you try to input a day, the panel would auto close when if you're done.
- If you reopen the date-picker, you can press enter to confirm your selection.
These are different from the React version.
fa40ad5
to
b47440a
Compare
@wendzhue However, 1 seems to be more subtle. A probable fix (which I am not really aware of) requiring an enter on changes could possibly disrupt the behavior of different components, because calendar-input is used in many places. Besides, it is not really an improvement but conformance to the react version, which is not necessarily 100% better. Lastly, it is not really that connected to the PR at hand. I think it is better to state 1 in a separate issue. |
b47440a
to
f13c5eb
Compare
Codecov Report
@@ Coverage Diff @@
## master #3804 +/- ##
==========================================
- Coverage 95.25% 95.24% -0.02%
==========================================
Files 726 726
Lines 14652 14679 +27
Branches 1907 1911 +4
==========================================
+ Hits 13957 13981 +24
- Misses 259 260 +1
- Partials 436 438 +2
Continue to review full report at Codecov.
|
f13c5eb
to
8d939cf
Compare
@wendzhue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rebase to the master branch.
8d939cf
to
7694ba9
Compare
It seems CI has failed with some strange 'Chromium disconnected' error. Rerun? |
Extracted the good parts suitable for a11y from NG-ZORRO#3146. See discussion there.
7694ba9
to
4db785f
Compare
The test job does not seem to be able to finish. (It is ok on my local machine.) |
…ORRO#3804) Extracted the good parts suitable for a11y from NG-ZORRO#3146. See discussion there.
…ORRO#3804) Extracted the good parts suitable for a11y from NG-ZORRO#3146. See discussion there.
Extracted the good parts suitable for a11y from #3146. See discussion there.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Opening date-picker with keyboard is not possible.
Opening date-picker by a click, does not focus on the inner input element.
Issue Number: N/A
What is the new behavior?
You can open date-picker pressing enter on the keyboard when the wrapper element is selected.
Once date-picker is open, the inner input element is selected, so it is immediately possible to input date from keyboard. (Of course, this is only applicable to the cases, when there is an inner input element: e.g., month and year pickers do not have any).
Does this PR introduce a breaking change?
Other information