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

feat: make input-based components open dialog on mobile device #1144

Merged
merged 38 commits into from
Feb 4, 2020
Merged

feat: make input-based components open dialog on mobile device #1144

merged 38 commits into from
Feb 4, 2020

Conversation

dimovpetar
Copy link
Contributor

@dimovpetar dimovpetar commented Jan 17, 2020

Created new ResponsivePopover component, which extends Popover. On desktop devices the behavior is the same as before. On phones a dialog is opened instead of popover. Same events and "opened" state apply.

Affected components:

  • ui5-input
  • ui5-select
  • ui5-combobox
  • ui5-multi-combobox
  • ui5-datepicker
  • ui5-tabcontainer

Fixes #1087

@claassistantio
Copy link

claassistantio commented Jan 17, 2020

CLA assistant check
All committers have signed the CLA.

@claassistantio
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@dimovpetar dimovpetar changed the title WIP(ui5-responsive-popover): initial implementation feat: make input-based components open dialog on mobile device Jan 29, 2020
@dimovpetar dimovpetar requested a review from vladitasev January 29, 2020 08:50
Copy link
Member

@ilhan007 ilhan007 left a comment

Choose a reason for hiding this comment

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

ReadOnly inputs can be edited, but should not. (Open the ComboBox page in mobile simulator and find the readonly combo, open it and edit it)

Copy link
Member

@ilhan007 ilhan007 left a comment

Choose a reason for hiding this comment

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

  • The ComboBox lacks autocompletion, that is present in desktop mode
  • Active state upon selection is missing in mobile mode

@dimovpetar
Copy link
Contributor Author

  • The ComboBox lacks autocompletion, that is present in desktop mode
  • Active state upon selection is missing in mobile mode
  1. fixed
  2. works ok after merging your change

@dimovpetar
Copy link
Contributor Author

ReadOnly inputs can be edited, but should not. (Open the ComboBox page in mobile simulator and find the readonly combo, open it and edit it)

fixed

@ilhan007 ilhan007 dismissed their stale review February 4, 2020 07:35

outdated

@@ -311,13 +325,17 @@ class ComboBox extends UI5Element {

_afterClosePopover() {
this._iconPressed = false;

if (isPhone()) {
this.blur();
Copy link
Member

@ilhan007 ilhan007 Feb 4, 2020

Choose a reason for hiding this comment

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

A little odd, what is the reason for "this.blur()", it is being opening again or?

Copy link
Contributor Author

@dimovpetar dimovpetar Feb 4, 2020

Choose a reason for hiding this comment

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

It is to close the device keyboard and prevent focus to stay in the ui5-combobox, so that the user can't write in it. The user should only be able to write in the dialog's input

this.firstRendering = false;
}

_afterOpenPopover() {
Copy link
Member

@ilhan007 ilhan007 Feb 4, 2020

Choose a reason for hiding this comment

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

move next to _afterClosePopover to improve readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


static async define(...params) {
await Promise.all([
Dialog.define(),
Copy link
Member

@ilhan007 ilhan007 Feb 4, 2020

Choose a reason for hiding this comment

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

if it`s one thing to await: "await Dialog.define();" is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@ilhan007 ilhan007 dismissed their stale review February 4, 2020 10:12

outdated

@ilhan007 ilhan007 merged commit d7b1179 into SAP:master Feb 4, 2020
@dimovpetar dimovpetar deleted the responsive_popover branch February 4, 2020 11:08
@MarekLukacAnodius
Copy link

No ResponsivePopover opened after pressing in ui5-input with suggestions. What is a expected behavior ?
Must click outside of input, then click again into input.

https://sap.github.io/ui5-webcomponents/playground/components/Input/
Mobil Tools in Browser Enabled

@dimovpetar
Copy link
Contributor Author

Hello @MarekLukacAnodius ,

The expected behavior is that an input with suggestions would automatically open a dialog on mobile device when tapped. I also noticed some issues, would you please open an issue for that?

Best regards,
Petar

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.

Enhance Mobile Support for Input like components
4 participants