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: switchKeyboardLayer #1954

Merged
merged 1 commit into from
May 1, 2023
Merged

fix: switchKeyboardLayer #1954

merged 1 commit into from
May 1, 2023

Conversation

eagleoflqj
Copy link
Contributor

#1357
This seems a regression after 0.90.
Test case provides the new way to switch keyboard layer by shift.

@arnog
Copy link
Owner

arnog commented May 1, 2023

Thanks for the PR. Could you tell me more about what was broken? How should I test this? BTW, as a side note, using the shift property is the recommended way to deal with multiple values for a key. Only use layers if you need a different layout (physical arrangement of the keycaps) between layers.

@eagleoflqj
Copy link
Contributor Author

The problem with [shift] is that I can't release the key when I want it to take effect. It's not possible to use it on mobile phone with single hand.
So as you can see in the e2e test I added, I define a layout with 2 layers - one for lower letters and the other for upper letters. Press shift key to switch and no need to hold it.
The bug with switchKeyboardLayer is that it expects layer.dataset.layer to be the name of the layer. However with current mathlive, the element doesn't contain data-layer attribute as it did in 0.89. So

if (layer.dataset.layer === layerName) {
doesn't work.
About how to test it, in any web page with a MathFieldElement, assign window.mathVirtualKeyboard.layouts as in the test case. The shift doesn't work in master's build, but works in this branch's build.

@arnog
Copy link
Owner

arnog commented May 1, 2023

Oh, I see. OK, thanks for the explanation and for tracking this down. That's a good fix.

That said, it is a bug that the shift key when used with the shift property only works while pressed. It should also work as a toggle. I'll look into it.

@arnog arnog merged commit 32271d7 into arnog:master May 1, 2023
@eagleoflqj eagleoflqj deleted the switch-layer branch May 1, 2023 15:18
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