-
Notifications
You must be signed in to change notification settings - Fork 27
Withdraw textPropertyForNode API, add removeKeyboardTypeForNode API #442
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
Changes from all commits
6ec1475
ce8078a
fb6d637
c5440bb
5f9e007
e7e3f2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -32,18 +32,19 @@ | |||||||||||||
| import com.gluonhq.attach.util.Util; | ||||||||||||||
| import javafx.animation.Interpolator; | ||||||||||||||
| import javafx.animation.TranslateTransition; | ||||||||||||||
| import javafx.application.Platform; | ||||||||||||||
| import javafx.beans.property.ReadOnlyFloatProperty; | ||||||||||||||
| import javafx.beans.property.ReadOnlyFloatWrapper; | ||||||||||||||
| import javafx.beans.property.ReadOnlyStringProperty; | ||||||||||||||
| import javafx.beans.property.ReadOnlyStringWrapper; | ||||||||||||||
| import javafx.beans.value.ChangeListener; | ||||||||||||||
| import javafx.beans.value.ObservableValue; | ||||||||||||||
| import javafx.scene.Node; | ||||||||||||||
| import javafx.scene.Parent; | ||||||||||||||
| import javafx.scene.input.MouseEvent; | ||||||||||||||
| import javafx.scene.Scene; | ||||||||||||||
| import javafx.util.Duration; | ||||||||||||||
|
|
||||||||||||||
| import java.util.HashMap; | ||||||||||||||
| import java.util.Collections; | ||||||||||||||
| import java.util.Map; | ||||||||||||||
| import java.util.Objects; | ||||||||||||||
| import java.util.Set; | ||||||||||||||
| import java.util.WeakHashMap; | ||||||||||||||
| import java.util.logging.Level; | ||||||||||||||
| import java.util.logging.Logger; | ||||||||||||||
|
|
@@ -60,75 +61,126 @@ public abstract class BaseKeyboardService implements KeyboardService { | |||||||||||||
| /** Map of nodes and keyboard types. */ | ||||||||||||||
| private final Map<Node, KeyboardType> nodeKeyboardTypes = new WeakHashMap<>(); | ||||||||||||||
|
|
||||||||||||||
| /** Map of nodes and text properties. */ | ||||||||||||||
| private static final Map<Node, ReadOnlyStringWrapper> nodeTextProperties = new WeakHashMap<>(); | ||||||||||||||
| /** Map of nodes to their visibility listeners. */ | ||||||||||||||
| private final Map<Node, ChangeListener<Number>> visibilityListeners = new WeakHashMap<>(); | ||||||||||||||
|
|
||||||||||||||
|
Comment on lines
+64
to
66
|
||||||||||||||
| /** Map of ids and nodes. */ | ||||||||||||||
| private static final Map<String, Node> idToNode = new HashMap<>(); | ||||||||||||||
| /** Scenes for which a focusOwner listener has already been installed. */ | ||||||||||||||
| private final Set<Scene> trackedScenes = Collections.newSetFromMap(new WeakHashMap<>()); | ||||||||||||||
|
|
||||||||||||||
| BaseKeyboardService() { | ||||||||||||||
| VISIBLE_HEIGHT.addListener((obs, ov, nv) -> { | ||||||||||||||
| if (nv != null && nv.doubleValue() <= 0) { | ||||||||||||||
| if (debug) { | ||||||||||||||
| LOG.info("Keyboard hidden, reset default type"); | ||||||||||||||
| } | ||||||||||||||
| applyActiveNodeId(""); // reset active node | ||||||||||||||
| applyKeyboardType(KeyboardType.ASCII.getValue()); | ||||||||||||||
| } | ||||||||||||||
| }); | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
70
to
+71
|
||||||||||||||
|
|
||||||||||||||
| @Override | ||||||||||||||
| public void keepVisibilityForNode(Node node) { | ||||||||||||||
| keepVisibilityForNode(node, null); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| @Override | ||||||||||||||
| public void keepVisibilityForNode(Node node, Parent parent) { | ||||||||||||||
| Objects.requireNonNull(node, "node must not be null"); | ||||||||||||||
| releaseVisibilityForNode(node); | ||||||||||||||
| ChangeListener<Number> listener = (obs, ov, nv) -> adjustPosition(node, parent, nv.doubleValue()); | ||||||||||||||
| visibilityListeners.put(node, listener); | ||||||||||||||
| VISIBLE_HEIGHT.addListener(listener); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| @Override | ||||||||||||||
| public void releaseVisibilityForNode(Node node) { | ||||||||||||||
| Objects.requireNonNull(node, "node must not be null"); | ||||||||||||||
| ChangeListener<Number> listener = visibilityListeners.remove(node); | ||||||||||||||
| if (listener != null) { | ||||||||||||||
| VISIBLE_HEIGHT.removeListener(listener); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| @Override | ||||||||||||||
| public ReadOnlyFloatProperty visibleHeightProperty() { | ||||||||||||||
| return VISIBLE_HEIGHT.getReadOnlyProperty(); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| @Override | ||||||||||||||
| public void setKeyboardTypeForNode(Node node, KeyboardType type) { | ||||||||||||||
| Objects.requireNonNull(node, "node must not be null"); | ||||||||||||||
| Objects.requireNonNull(type, "type must not be null"); | ||||||||||||||
| nodeKeyboardTypes.put(node, type); | ||||||||||||||
| installEventFilter(node); | ||||||||||||||
| attachFocusTracker(node); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| @Override | ||||||||||||||
| public ReadOnlyStringProperty textPropertyForNode(Node node) { | ||||||||||||||
| public void removeKeyboardTypeForNode(Node node) { | ||||||||||||||
| Objects.requireNonNull(node, "node must not be null"); | ||||||||||||||
| installEventFilter(node); | ||||||||||||||
| return nodeTextProperties.computeIfAbsent(node, n -> { | ||||||||||||||
| idToNode.put(syntheticId(n), n); | ||||||||||||||
| return new ReadOnlyStringWrapper(""); | ||||||||||||||
| }).getReadOnlyProperty(); | ||||||||||||||
| nodeKeyboardTypes.remove(node); | ||||||||||||||
|
||||||||||||||
| nodeKeyboardTypes.remove(node); | |
| nodeKeyboardTypes.remove(node); | |
| Scene scene = node.getScene(); | |
| if (scene != null && scene.getFocusOwner() == node) { | |
| applyTypeFor(node); | |
| } |
Copilot
AI
Apr 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When tracking a Scene for the first time, the current focus owner’s keyboard type/id isn’t applied unless it happens to be the node being registered. Since focusOwnerProperty() listeners don’t fire on registration, the native layer can remain out of sync until the next focus change. Consider applying applyTypeFor(scene.getFocusOwner()) right after installing the listener (and similarly after trackScene(nv) in the deferred branch).
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,6 @@ static jclass jKeyboardServiceClass; | |
| static jclass jAttachKeyboardClass; | ||
| static jclass jActivityClass; | ||
| static jmethodID jAttach_notifyHeightMethod; | ||
| static jmethodID jAttach_notifyComposingTextMethod; | ||
| static jmethodID jActivity_setKeyboardTypeMethod; | ||
| static jmethodID jActivity_setActiveNodeIdMethod; | ||
|
|
||
|
|
@@ -53,7 +52,6 @@ JNI_OnLoad_keyboard(JavaVM *vm, void *reserved) | |
| ATTACH_LOG_FINE("Initializing native Keyboard from OnLoad"); | ||
| jAttachKeyboardClass = (*env)->NewGlobalRef(env, (*env)->FindClass(env, "com/gluonhq/attach/keyboard/impl/AndroidKeyboardService")); | ||
| jAttach_notifyHeightMethod = (*env)->GetStaticMethodID(env, jAttachKeyboardClass, "notifyVisibleHeight", "(F)V"); | ||
| jAttach_notifyComposingTextMethod = (*env)->GetStaticMethodID(env, jAttachKeyboardClass, "notifyComposingText", "(Ljava/lang/String;Ljava/lang/String;)V"); | ||
| initKeyboard(); | ||
|
Comment on lines
52
to
55
|
||
| ATTACH_LOG_FINE("Initializing native Keyboard done"); | ||
| return JNI_VERSION_1_8; | ||
|
|
@@ -115,23 +113,6 @@ JNIEXPORT void JNICALL Java_com_gluonhq_attach_keyboard_impl_AndroidKeyboardServ | |
| ATTACH_LOG_FINE("nativeSetActiveNodeId done"); | ||
| } | ||
|
|
||
| ////////////////////////////////// | ||
| // native (Substrate) to Java // | ||
| ////////////////////////////////// | ||
|
|
||
| void attach_setComposingText(const char *id, const char *text) | ||
| { | ||
| ATTACH_LOG_FINE("attach_setComposingText: forwarding to Graal: id=%s, text=%s", id, text); | ||
| ATTACH_GRAAL(); | ||
| jstring graalId = (*graalEnv)->NewStringUTF(graalEnv, id); | ||
| jstring graalText = (*graalEnv)->NewStringUTF(graalEnv, text); | ||
| (*graalEnv)->CallStaticVoidMethod(graalEnv, jAttachKeyboardClass, jAttach_notifyComposingTextMethod, graalId, graalText); | ||
| (*graalEnv)->DeleteLocalRef(graalEnv, graalText); | ||
| (*graalEnv)->DeleteLocalRef(graalEnv, graalId); | ||
| DETACH_GRAAL(); | ||
| ATTACH_LOG_FINE("attach_setComposingText done"); | ||
| } | ||
|
|
||
| /////////////////////////// | ||
| // From Dalvik to native // | ||
| /////////////////////////// | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Javadoc says this removes an “event filter”, but the implementation now uses a scene-level
focusOwnerProperty()listener rather than an event filter on the node. Please update the documentation to describe what is actually installed/removed (and what the lifecycle expectations are).