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.

@nallwhy
Copy link
Contributor Author

nallwhy commented Feb 21, 2025

@SteffenDE @chrismccord

I wanted to check in on the status of this PR. The last update mentioned that Chris wanted to look into it further. Has there been any progress or any additional feedback?

Let me know if there’s anything else I can do to help move it forward.

Thanks!

@nallwhy
Copy link
Contributor Author

nallwhy commented Feb 21, 2025

I wanted to provide some additional context regarding this PR and the need for firePhxChangeWhileComposing.

Unlike Japanese romaji or Chinese pinyin keyboards, which convert alphabetic input into their respective language’s characters (e.g. k (invalid) → ka (invalid) → か (valid)), Korean uses a native input system where every stage of composition results in a valid character (e.g. ㄱ (valid) → 가 (valid) → 각 (valid))

Each intermediate state represents a complete and meaningful character in itself. Furthermore, since every step produces a valid character, it’s impossible to determine whether the composition has finished until the next character starts being typed.

Because of this, while Korean technically has a composition phase, phx-change events must still be triggered at every step. Without firing phx-change during composition, issues arise—such as the last character not being included in cases where an event (e.g., a search trigger) relies on the updated input.

This is why the default behavior of skipping phx-change during composition is problematic for Korean users. The addition of firePhxChangeWhileComposing provides flexibility, allowing developers to enable phx-change during composition when needed, without disrupting other language input behaviors.

Let me know if any further clarification is needed. Thanks again for reviewing this PR!

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