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

WIP: Maliit 3 major refactoring #108

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Conversation

dobey
Copy link
Contributor

@dobey dobey commented May 24, 2022

No description provided.

@dobey
Copy link
Contributor Author

dobey commented May 24, 2022

I've made this draft PR and requested reviews from @mikhas @jpetersen and @pvuorela to get some feedback and make sure the way I'm going with this is the right way forward for getting rid of complex, old, and unused API, while some new API will be added in to allow keyboard plugins to implement better handling across a broader range of devices (such as using a gamepad controller to input text on a large TV), and allow for more interesting user experiences in more traditional scenarios.

@pvuorela
Copy link
Contributor

For very quick glance it's looking nice. Only detail I'm immediately pondering is whether the setLanguage part should be kept. What that code does is allowing empty text field cursor to follow expected text direction. That is for example: device locale in arabic, user has empty text field focused, changes keyboard layout to english. Now by locale the cursor would go to the right hand side of the field and then the first input character would have it jump to the left. With the text direction known the cursor can switch position right after the keyboard layout change.

For above the text direction from input method side would suffice, but then again the input locale is also exposed in QInputMethod API.

@dobey
Copy link
Contributor Author

dobey commented May 24, 2022

Now by locale the cursor would go to the right hand side of the field and then the first input character would have it jump to the left.

But, we cannot change the input direction from the keyboard side, so if I change between RTL and LTR languages in maliit, and sometimes people need to mix languages in the middle of sentences, too. So I think it will be problematic regardless. Though if there is something we need to do more proactively here, perhaps we can do it in a better way now, too. Hopefully such things will be more apparent when this branch gets nearer to being ready to merge.

But for now, I'm also trying to remove things piecemeal so that if something is off in the direction things are headed here, it should be easier to recover smaller things like this if necessary.

@pvuorela
Copy link
Contributor

This is only fallback when the text direction cannot be inferred from the text content. Mostly deciding cursor position on an empty text field.

For reference https://github.com/qt/qtdeclarative/blob/dev/src/quick/items/qquicktextedit.cpp#L808

@dobey dobey force-pushed the maliit-3-refactor branch 2 times, most recently from 0126362 to 4a5ad86 Compare June 8, 2022 20:17
@@ -44,6 +44,9 @@ namespace Maliit {
//! only integer numbers allowed
NumberContentType,

//! only numbers and formatted characters
FormattedNumberContentType,
Copy link
Contributor

Choose a reason for hiding this comment

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

Pondering if we should just use Qt::inputMethodHint which offers more types and hint combinations.

On the history side not entirely sure did we even add this before Qt got that hint of its own.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how much sense it makes, because the Wayland protocols are so unstable right now, and Phosh devs keep trying to push their own changes which break the existing protocols, and Qt::InputMethodHints doesn't have flags for everything, so I can only see it getting more complicated to keep updated, along with I don't want to have to block on getting things into Qt, maybe backported, etc… if we want to add anything else in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could alternatively be a similar enum. But then again Qt::InputMethodHints should not block adding new extensions, those can be presented with other properties. We already do pass the content type here but also the input method hints.

Actually looks like I've halfway marked this as deprecated in 2013 with the Qt5 port. minputcontext.cpp / MInputContext::getStateInformation()

Also curious on what those wayland changes would be. The input hint side has seemed relatively stable for many years.

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.

2 participants