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 dialog calculations #346

Closed
wants to merge 8 commits into from
Closed

Fix dialog calculations #346

wants to merge 8 commits into from

Conversation

mario
Copy link
Collaborator

@mario mario commented Feb 19, 2019

The emoji keyboard wasn't working as expected when used inside a dialog. This PR fixes that.

Please test both regular and dialog to make sure I didn't break anything :)

Please note that I've based this on an AndroidX branch, but it could easily be back-ported, if needed.

Signed-off-by: Mario Danic <mario@lovelyhq.com>
Signed-off-by: Mario Danic <mario@lovelyhq.com>
Signed-off-by: Mario Danic <mario@lovelyhq.com>
Signed-off-by: Mario Danic <mario@lovelyhq.com>
Signed-off-by: Mario Danic <mario@lovelyhq.com>
Signed-off-by: Mario Danic <mario@lovelyhq.com>
Signed-off-by: Mario Danic <mario@lovelyhq.com>
Copy link
Owner

@vanniktech vanniktech left a comment

Choose a reason for hiding this comment

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

I should probably know this but since when do we have that animation? :D It's only present for me in the main screen and not for the dialog.

Can't we slide it in from the top to the bottom without this skewing?

@mario
Copy link
Collaborator Author

mario commented Feb 19, 2019

Since you merged my PRs that added the animations :P

And yes, the dialog doesn't have it, though it could if you want! :D

It's an option btw, so you don't have to use it but it's there as a sample.

@vanniktech
Copy link
Owner

Yeah, I know it's configurable. Maybe we can choose a better default for the demo app nonetheless though. :D

@vanniktech
Copy link
Owner

Maybe wait for @rubengees approval here since he usually has a device or two that needs testing.

@mario
Copy link
Collaborator Author

mario commented Feb 19, 2019

@vanniktech I look forward to your PR :P

@vanniktech
Copy link
Owner

@mario well played!

@mario
Copy link
Collaborator Author

mario commented Feb 23, 2019

@rubengees did you maybe have the time to check this out?

@mario
Copy link
Collaborator Author

mario commented Mar 5, 2019

@rubengees I know you're around here somewhere :p

@rubengees
Copy link
Collaborator

@mario Yeah, will look into it now :)

Copy link
Collaborator

@rubengees rubengees left a comment

Choose a reason for hiding this comment

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

The code changes look fine and work perfectly in the normal case. The picker for the dialog is like this though (on my usual device OP5T Stock@9.0):

screenshot_20190306-001700

@mario
Copy link
Collaborator Author

mario commented Mar 5, 2019 via email

@rubengees
Copy link
Collaborator

Hmm, maybe I checked the wrong branch? Will also confirm tomorrow.

@vanniktech
Copy link
Owner

What's the state here?

@mario
Copy link
Collaborator Author

mario commented Mar 9, 2019

Pending another check by both me and @rubengees.

@rubengees
Copy link
Collaborator

@mario Just double checked, I can still reproduce such weird behaviour. :/

@vanniktech
Copy link
Owner

@mario do you want to pick this up again?

@mario
Copy link
Collaborator Author

mario commented Apr 30, 2019 via email

@vanniktech
Copy link
Owner

@mario friendly ping since now I'm active :D

@mario
Copy link
Collaborator Author

mario commented Sep 10, 2019 via email

@vanniktech
Copy link
Owner

https://media.giphy.com/media/l2Je4SgTZDofg5MfC/giphy.gif

@mario
Copy link
Collaborator Author

mario commented Sep 24, 2019

Closing in favor of a new PR.

@mario mario closed this Sep 24, 2019
@mario mario deleted the fix-dialog-calculations branch September 24, 2019 12:42
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.

None yet

3 participants