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: Added option for sticky display of variants panel with a single click #2554

Merged
merged 7 commits into from
Nov 29, 2024

Conversation

konbu310
Copy link
Contributor

@konbu310 konbu310 commented Nov 27, 2024

fix #2549

Overview

This PR adds a stickyVariantPanel option to the VirtualKeyboardKeycap. When this option is enabled, clicking on a key with variants configured will display the Variants Panel without a long press. Additionally, the panel will not automatically hide.

The motivation for implementing this feature is described in #2549, and a similar request can be found in #2363.

Notes

  • Tests have not been added yet.
  • Added an example: sticky-variant-panel/index.html.
  • I am not entirely confident about the API naming.

Copy link
Owner

@arnog arnog left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
Looks good overall, but see two inline comments.

@@ -1135,19 +1135,21 @@ function handlePointerDown(ev: PointerEvent) {
}

if (keycap.variants) {
if (pressAndHoldTimer) clearTimeout(pressAndHoldTimer);
Copy link
Owner

Choose a reason for hiding this comment

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

Could you explain why clearing the timer had to be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like I mistakenly removed it, so I’ll restore it.

@@ -98,6 +98,9 @@ export interface VirtualKeyboardKeycap {

/** Name of the layer to shift to when the key is pressed */
layer: string;

/** Open variants panel without long press and does not close automatically */
popover: boolean;
Copy link
Owner

Choose a reason for hiding this comment

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

I would probably go with something a bit more descriptive. Maybe stickyVariantPanel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good name.

@konbu310 konbu310 changed the title feat: Added option for fixed display of Variants Panel with a single click feat: Added option for stickey display of variants panel with a single click Nov 28, 2024
@konbu310 konbu310 requested a review from arnog November 28, 2024 02:55
@konbu310 konbu310 changed the title feat: Added option for stickey display of variants panel with a single click feat: Added option for sticky display of variants panel with a single click Nov 28, 2024
Copy link
Owner

@arnog arnog left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@arnog arnog merged commit 5622a8e into arnog:master Nov 29, 2024
2 checks 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.

Feature Request: Variants panel that can be persistently displayed with a single click
2 participants