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

Add firePhxChangeWhileComposing opt to live_socket #3582

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nallwhy
Copy link
Contributor

@nallwhy nallwhy commented Dec 18, 2024

Through issue #3322, I observed the problem with Safari and IME composition for languages like Japanese, where firing phx-change during a composition session can cause unexpected behavior. However, for Korean input, the current behavior of skipping phx-change during composition introduces its own issues.

In Korean, even while composing, each intermediate character (e.g., 가) is considered a meaningful and complete character, even though it may later change to another (e.g., 각). As a result, treating intermediate characters as valid inputs and firing phx-change during composition is crucial for proper functionality. The current default behavior of skipping phx-change during isComposing does not align with the needs of Korean input users and would likely not be desirable for anyone using Korean.

To address this, I’ve added an explicit configuration option, opts.firePhxChangeWhileComposing, allowing developers to control whether phx-change should fire during composition sessions. While the default remains unchanged(�false), this option enables developers to adjust the behavior based on their application’s language and input requirements.

This change ensures greater flexibility for IME-based input scenarios, particularly for languages like Korean where the current behavior is problematic.

@SteffenDE
Copy link
Collaborator

Hey @nallwhy,

thank you very much for the contribution. I have no knowledge at all about how Koreans type, so this is very much appreciated. Can you please remove the changes to the assets in the priv folder? Those are built automatically once this is merged :)

@nallwhy nallwhy force-pushed the add_firePhxChangeWhileComposing_opt_to_live_socket branch from e0fd81b to ec682b8 Compare December 18, 2024 09:34
@nallwhy
Copy link
Contributor Author

nallwhy commented Dec 18, 2024

@SteffenDE
Thank you for the feedback! I’ve removed the changes to the assets in the priv folder as you suggested.

@SteffenDE
Copy link
Collaborator

It would be nice if we were able to remove the special handling once Safari is fixed. The last question I have is: if you set the flag, isn’t compositing broken in Safari for Korean users?

@nallwhy
Copy link
Contributor Author

nallwhy commented Dec 18, 2024

It doesn’t cause any issues in Korean. Using the text as-is in the composing state is natural for Korean users.
Most Koreans would likely enable this option and use it by default.

@SteffenDE
Copy link
Collaborator

just a quick update here: Chris wanted to look into this a little more himself

@nallwhy
Copy link
Contributor Author

nallwhy commented Dec 27, 2024

Since this behavior has already been implemented, it may be difficult to change it for backward compatibility reasons. However, the best approach would be to provide an option to disable the trigger during composition. This would allow users to explicitly disable it only in specific cases, such as with the Japanese + Safari combination, while ensuring it works naturally in all other scenarios.

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