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

Space swipe to toggle numpad layout, with side bonuses #950

Merged
merged 16 commits into from
Jul 9, 2024

Conversation

devycarol
Copy link
Contributor

This patch was honestly just a quality-of-life improvement for test-driving the D-pad toy, but it turns out this is pretty nifty to have for the numpad in addition to (eventually) the D-pad. It will greatly simplify cloning the numpad later on.

I've added a space-swipe gesture to toggle the numpad. It's one-and-done, so you don't need to worry about it rapidly changing back and between. Would you like me to make the language slide work the same way? I feel it cycles layouts way too rapidly, to the point where "landing" on the right one is super tedious. Whereas repeated swipes are not that difficult to do, especially since most people have at most 2 languages.

This required implementing toggling logic for the numpad. A side effect of that is the numpad key and symbols long-press can now be used for sliding input—the latter actually had a bug beforehand where it'd show the trail, but not return to the previous layout on release. It works pretty flawlessly now as far as I can tell.

I've proposed a change to the bottom row of the numpad to take advantage of this:

  • The 'symbols' key is replaced with 'numpad', matching its predecessor layout.
  • Key-widths, likewise, are adjusted to match.
  • 'Equals' has the functional background color now. I plan on putting the (functional) d-pad key symmetrical to the numpad one in the default symbols layout, so that should line up nicely. Only giving numbers the numeric/character background is best for legibility in my opinion.

Etc:

If you wish to apply the step-minimum from the language slide to the numpad one and/or a one-shot implementation of the language slide, it'd require sizeable re-tooling in a way that'd be hard to keep track of and migrate to KeyboardActionListener later on (as far as I can tell). I don't think that's necessary, since the fast-typing cooldown applies across the board.

As I implemented the toggling logic, I noticed but didn't address that the emoji and clipboard layouts don't have any sort of state-memory—they always return to the non-shift-locked alphabet layout. This can be annoying if you enter the clipboard layout from the numpad and then hit the 'exit' button, for example.

@Helium314
Copy link
Owner

I've proposed a change to the bottom row of the numpad to take advantage of this:

  • The 'symbols' key is replaced with 'numpad', matching its predecessor layout.
  • Key-widths, likewise, are adjusted to match.
  • 'Equals' has the functional background color now. I plan on putting the (functional) d-pad key symmetrical to the numpad one in the default symbols layout, so that should line up nicely. Only giving numbers the numeric/character background is best for legibility in my opinion.

@BlackyHawky you implemented the numpad and probably put a lot of work / testing in the layout. Do you have any comment / opinion on this?
From my side the changes are ok, but I barely use the numpad.

Would you like me to make the language slide work the same way?

There is still #656, which I haven't even properly looked at....

@devycarol
Copy link
Contributor Author

devycarol commented Jul 7, 2024

The toggling logic also allows for a toolbar key, so I've added that as well.

Across the board, the fallback behavior in the phone and number keypads seems to work just fine—layout just doesn't change.

@devycarol devycarol marked this pull request as draft July 7, 2024 23:35
@devycarol
Copy link
Contributor Author

Bug: the toggling breaks when the state is reloaded from, say, a change of screen orientation. Working on a fix.

@devycarol devycarol marked this pull request as ready for review July 8, 2024 00:14
@BlackyHawky
Copy link
Contributor

@Helium314
First of all, thank you for asking my opinion.

@devycarol
The ability to change view by swiping the space bar is a great idea. 👍
By the way, for even more personalization, perhaps in the future we can extend this functionality to the view of emojis, symbols and so on. 🤔

Just a few comments:

  • The 'symbols' key is replaced with 'numpad', matching its predecessor layout.

    Why? I'm not sure it respects the pola...
    Indeed, this button is used to display the "numpad" view, not the "symbol" view.
    Maybe I missed something...

  • Key-widths, likewise, are adjusted to match.

    I much prefer the alignment defined previously; is it a matter of taste or is it useful for some other reason?

@devycarol
Copy link
Contributor Author

devycarol commented Jul 8, 2024

I think that's a good point. Layout-switch keys universally display the layout they'll go to when clicked, at least in the main keyboard view. I guess I got carried away with what I implemented 😅

Toggling will remain used for the toolbar key, but it's unnecessary to use that key in the numpad layout itself since there are already 2 other toggling methods. And on further reflection, "the key does different things depending on how you got here" is not in good taste for main layouts.

There was an idea of "later, add the ability to lock-into the layout via the toggle key during sliding input from a different one," but that'd require a significant hard-to-keep-track-of re-tooling that's—again—unnecessary, since there are already numerous other toggling mechanisms.

I'll change it back to 'symbols', sliding input should still work.

I much prefer the alignment defined previously; is it a matter of taste or is it useful for some other reason?

Part of it was to ensure the numpad key stayed (somewhat) aligned with its previous position, which is now moot. The other reason is that variate widths for keys in the bottom row can be disruptive, but with the other change eliminated I don't think the justification is strong enough. There's a case about equally as good to ensure the widths align with the rows above, so I'll go ahead and revert that as well.

@devycarol devycarol marked this pull request as draft July 8, 2024 13:18
@BlackyHawky
Copy link
Contributor

Great, I'll let you continue with your work.
If you need an opinion, don't hesitate.

@devycarol devycarol marked this pull request as ready for review July 8, 2024 16:31
@devycarol
Copy link
Contributor Author

Thanks for stopping by!

@devycarol
Copy link
Contributor Author

devycarol commented Jul 8, 2024

@BlackyHawky Oh, but the key colors.

Could we change the background of the 'symbols' and 'equals' keys to the functional type? I think the keys should only be character-colored if they're numbers. Making the symbol key 'functional' better matches its predecessor the numpad key, and 'equals' will align better with the d-pad key once it's added. (And its fellow symbols are also functional.)

@BlackyHawky
Copy link
Contributor

Really sorry, I forgot to update my comment above. 😅

I share your opinion of changing the color of these keys. 😉

@devycarol
Copy link
Contributor Author

Alrighty! I don't anticipate any further changes on my part.

@Helium314
Copy link
Owner

Will have a look tomorrow (evening / Europe time).
Today I don't want to see any more code, after coming home from work and then working 6 hours straight on another app...

@@ -879,6 +880,13 @@ private void dragFingerOutFromOldKey(final Key oldKey, final int x, final int y)
}
}

private boolean oneShotSwipe(final int swipeSetting) {
Copy link
Owner

Choose a reason for hiding this comment

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

The one-shot swipe logic seems to be more suitable for KeyboardActionListenerImpl.
Is it not in there because there is no callback when a key swipe has ended (except for delete)?

@Helium314
Copy link
Owner

If you wish to apply the step-minimum from the language slide to the numpad one and/or a one-shot implementation of the language slide, it'd require sizeable re-tooling in a way that'd be hard to keep track of and migrate to KeyboardActionListener later on (as far as I can tell). I don't think that's necessary, since the fast-typing cooldown applies across the board.

I think a step minimum for the numpad toggle would be good, but it looks like a main issue is that in KeyboardActionListener you don't know when the swipe was ended. Is this correct?

@Helium314
Copy link
Owner

As I implemented the toggling logic, I noticed but didn't address that the emoji and clipboard layouts don't have any sort of state-memory—they always return to the non-shift-locked alphabet layout. This can be annoying if you enter the clipboard layout from the numpad and then hit the 'exit' button, for example.

I never properly looked into this, but I've always had the impression that the KeyboardState is much more complicated than it needs to be.
If this is the case, then I think such state-memory for clipboard could be added as part of simplifying the KeyboardState. In the past I've done such simplification with a few classes when migrating them to Kotlin.

@devycarol
Copy link
Contributor Author

I think a step minimum for the numpad toggle would be good, but it looks like a main issue is that in KeyboardActionListener you don't know when the swipe was ended. Is this correct?

Yes.

@Helium314
Copy link
Owner

Yes.

Possibly onUpWithDeletePointerActive could be changed to a more generic function like onEndSwipe(keyCode).

But maybe this should be done in a separate PR, when making the key swipe part of the interface more generic. Keeping track of (is-in-swipe) states in KeyboardActionListener at first glance appears to do the trick, but needs to consider that there can be multiple pointer trackers and multiple swipes simultaneously.

@devycarol
Copy link
Contributor Author

devycarol commented Jul 9, 2024

Right. Using the delete slider at the same time as the forward-delete slider is the funnest thing in the world 😅
Coming soon :)

I think you're right about how to migrate that logic. It's something I could do later, unless you really want a step minimum right away. I just haven't had any issues without one, and it'd be one more thing to keep track of in the pointer state that I'd enjoy not worrying about.

@devycarol
Copy link
Contributor Author

devycarol commented Jul 9, 2024

How would that be done by the way, a JSON property like "horizontalSwipe"? And then it'd override sliding input and app settings? Curious if app settings would even need to change that much.

I also see the possibility of an omnidirectional swipe. I once experimented with that on the trackpad slider and it was mad fun but also mad buggy. Intense zig-zagging, frequent crashes...

That'd require some way of knowing where visual line-breaks are, not just line-ends. I know iOS has a 1:1 thing for moving the cursor in a text box.

@Helium314
Copy link
Owner

I think you're right about how to migrate that logic. It's something I could do later, unless you really want a step minimum right away. I just haven't had any issues without one, and it'd be one more thing to keep track of in the pointer state that I'd enjoy not worrying about.

Step minimum is not necessary right now. I think it would be good, but not needed. And implementing it now would mostly be a waste of time.
I think I'll open an issue on the adjustments I had in mind for the key swipe interface.

How would that be done by the way, a JSON property like "horizontalSwipe"? And then it'd override sliding input and app settings? Curious if app settings would even need to change that much.

I haven't put much thought here. So far my idea was to add a setting that would switch from popup keys to unexpected keyboard style (with some limitations).
Ideally it would also allow for custom (key-independent) actions like in #532 (might be simple by ignoring the key code and only using direction).

@Helium314 Helium314 merged commit 105d044 into Helium314:main Jul 9, 2024
1 check passed
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.

3 participants