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

fix: enable input into autoOpenDisabled date picker on mobile #3165

Merged
merged 13 commits into from
Dec 15, 2021

Conversation

tomivirkki
Copy link
Member

@tomivirkki tomivirkki commented Dec 8, 2021

This change enables typing text to the date picker input field on small screen devices (also iOS generally) when auto-open is disabled and the dropdown is closed.

Fixes #3047

Kapture.2021-12-10.at.12.05.51.mp4

Note: Currently, text input is generally disabled on iOS to avoid the virtual keyboard from covering the date picker dropdown:

Screenshot 2021-12-10 at 11 56 09

After this change, when auto-open is disabled, you can focus the input and type into the field also on iOS while the dropdown remains closed.

Screenshot 2021-12-10 at 11 59 37

As soon as the dropdown opens, the input field is blurred, even on larger screen touch devices such as iPads.

Screenshot 2021-12-10 at 12 00 04

@tomivirkki tomivirkki marked this pull request as draft December 8, 2021 15:07
@tomivirkki tomivirkki force-pushed the date-picker-mobile-auto-open-disabled branch 4 times, most recently from 27e156b to 963ca6f Compare December 10, 2021 08:34
@tomivirkki tomivirkki force-pushed the date-picker-mobile-auto-open-disabled branch 2 times, most recently from 5c611b2 to 4a6bbe6 Compare December 13, 2021 08:33
@jouni
Copy link
Member

jouni commented Dec 13, 2021

Looks great! The only issue I noticed is that when you have the overlay open, and click outside it or click "Cancel" or "Today", the keyboard pops up again, because the field gets focused. I think I would prefer to have the field blurred in that case instead. Not sure how much complexity that might add.

@tomivirkki
Copy link
Member Author

The input refocus logic comes from #1901 (a recent a11y fix)

@tomivirkki tomivirkki force-pushed the date-picker-mobile-auto-open-disabled branch from 4a6bbe6 to f604d7e Compare December 13, 2021 09:47
Comment on lines -423 to -428

this.addEventListener('touchend', (e) => {
if (!this._isClearButton(e)) {
e.preventDefault();
}
});
Copy link
Member Author

Choose a reason for hiding this comment

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

Canceling the "touchend" events seems not to be relevant anymore. It was originally added 6 years ago and was probably related to preventing ghost clicks.

@@ -96,14 +112,21 @@ describe('basic features', () => {
await oneEvent(datepicker.$.overlay, 'vaadin-overlay-open');
});

it('should not open on input tap when autoOpenDisabled is true and not on mobile', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This test was moved inside "auto open disabled"

@@ -188,12 +211,6 @@ describe('basic features', () => {
await oneEvent(datepicker.$.overlay, 'vaadin-overlay-open');
});

it('should open by tapping the calendar icon even if autoOpenDisabled is true', async () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This test was moved inside "auto open disabled"

@tomivirkki tomivirkki marked this pull request as ready for review December 13, 2021 10:43
Copy link
Member

@web-padawan web-padawan left a comment

Choose a reason for hiding this comment

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

Tested on iOS Simulator, works fine 👍

@sonarcloud
Copy link

sonarcloud bot commented Dec 15, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@vursen vursen left a comment

Choose a reason for hiding this comment

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

Tested the feature on iPhone and it works perfectly with perhaps one UX caveat: the keyboard pops up again after the calendar is closed, @jouni has already mentioned this:

The only issue I noticed is that when you have the overlay open, and click outside it or click "Cancel" or "Today", the keyboard pops up again, because the field gets focused

However, it is probably a necessary evil as otherwise, we lose focus after closing the calendar which makes it difficult to interact with the date-picker by the keyboard for screen reader users.

@tomivirkki tomivirkki merged commit bccc116 into master Dec 15, 2021
@tomivirkki tomivirkki deleted the date-picker-mobile-auto-open-disabled branch December 15, 2021 08:33
@tomivirkki
Copy link
Member Author

The only issue I noticed is that when you have the overlay open, and click outside it or click "Cancel" or "Today", the keyboard pops up again, because the field gets focused

@jouni @vursen
Created a separate issue for this addressing combo-box and date-picker #3191

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with platform 23.0.0.alpha1 and is also targeting the upcoming stable 23.0.0 version.

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

Successfully merging this pull request may close these issues.

Enable DatePicker to disable AutoOpen on touch devices
5 participants