Skip to content

Comments

Implement click-to-clear-focus#2533

Merged
m-sasha merged 6 commits intojb-mainfrom
m-sasha/click-to-clear-focus
Nov 24, 2025
Merged

Implement click-to-clear-focus#2533
m-sasha merged 6 commits intojb-mainfrom
m-sasha/click-to-clear-focus

Conversation

@m-sasha
Copy link

@m-sasha m-sasha commented Oct 28, 2025

Originally implemented in AOSP here

Fixes https://youtrack.jetbrains.com/issue/CMP-8891/Implement-Click-To-Clear-focus

Testing

Added a unit test and also tested manually with a few text fields.

Release Notes

Features - Multiple Platforms

  • Clicking outside of any focusable node using the mouse will now clear focus from the currently focused node, if any. This behavior can be disabled by setting isClearFocusOnMouseDownEnabled = false in ComposePanel, ComposeWindow or ComposeDialog.

@m-sasha m-sasha force-pushed the m-sasha/click-to-clear-focus branch 4 times, most recently from eb5e8c2 to ab6011d Compare November 4, 2025 13:13
@m-sasha m-sasha requested a review from igordmn November 4, 2025 14:16
@igordmn igordmn requested a review from MatkovIvan November 5, 2025 12:08
@igordmn
Copy link
Collaborator

igordmn commented Nov 5, 2025

Needs 2 approvals, as it has new public API (new experimental flag)

Copy link
Member

@MatkovIvan MatkovIvan left a comment

Choose a reason for hiding this comment

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

LGTM, but blocking until we create 1.10 branch

var ComposeUiFlags.useLegacyRenderNodeLayers by SkikoComposeUiFlags::useLegacyRenderNodeLayers

/** This flag enables clearing focus on pointer down by default. */
@ExperimentalComposeUiApi
Copy link
Collaborator

@igordmn igordmn Nov 5, 2025

Choose a reason for hiding this comment

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

Please create a task to remove Experimental with target 1.13

Copy link
Author

Choose a reason for hiding this comment

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

Shouldn't we remove the flag itself, in 1.12 or so?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, we need to:

  • remove the flag itself from ComposeUiFlags
  • keep the ComposePanel/ComposeWindow property, remove Experimental from it
  • do that in 1.13, as we release this API in 1.11

Copy link
Author

Choose a reason for hiding this comment

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


/** This flag enables clearing focus on pointer down by default. */
@ExperimentalComposeUiApi
var ComposeUiFlags.isClearFocusOnPointerDownEnabled by SkikoComposeUiFlags::isClearFocusOnPointerDownEnabled
Copy link
Collaborator

Choose a reason for hiding this comment

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

Android moved this flag to a ComposeView property.

It is probably legit to set different values for different cases, so I propose to do the same in ComposeWindow, ComposePanel and Window composables (still keep it experimental).

Cases in my mind where you don't want to clear focus:

  • IntelliJ interop
  • text editors (keep focus in editor when you click on a toolbar or a scrollbar)

Copy link
Author

Choose a reason for hiding this comment

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

The value needs to be available in BaseComposeScene. What is the correct way to pass it there?

Copy link
Member

Choose a reason for hiding this comment

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

SceneContext or PlatformContext

Copy link
Author

Choose a reason for hiding this comment

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

Note that this is not only a desktop feature, so we will need flags in iOS and web interop entry points.

Copy link
Member

Choose a reason for hiding this comment

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

It should provide working-no-op impl and must keep optional for platforms to implement

Copy link
Collaborator

@igordmn igordmn Nov 5, 2025

Choose a reason for hiding this comment

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

IntelliJ interop

If we want to provide seamless interop, we need to keep this ability, as It doesn't clear focus on an empty space (open Settings, focus the Search field, click on an empty space).

text editors

Not so sure if it is right to configure it on global level, but from UX perspective It may look bad when the app loses focus out of the editor when I press on scrollbar, statusbar or toolbar (for example to switch to Bold)

If it's not standard desktop behavior to clear focus, then indeed we should allow both options.

I checked native apps and:

  • it clears focus in WinUI (new Windows API) apps
  • it doesn't clear focus in Win32 apps (old Windows API)
  • it doesn't clear focus in AppKit apps

My experiments:

  • Windows (doesn't clear)

    • File properties
    • Notepad
    • Nvidia Control Panel (seems uses Win32 API)
  • Windows (clears)

    • Settings
    • Paint
  • macOS (doesn't clear)

    • Settings
    • File properties
    • TextEdit save window
    • Choose file dialog (Search field)
    • Grapher
    • Activity monitor main table
  • macOS (clears)

    • Activity monitor search field
    • Finder search field

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we go with the logic that behaviors need to be controlled per entry point, we should allow controlling from the entry point most behaviors currently controlled by global flags

We can follow the strategy - if we think the lack of configurability can introduce significant flaws, then we need to add configurability. Lack of configurability of the first focused element seems doesn't introduce significant flaws.

Copy link
Author

Choose a reason for hiding this comment

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

It's not about the configurability itself, but about the granularity at which we need the configurability. Is per-application configurability enough, or do we need something more granular? Per window? Per entry point? Per element?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is per-application configurability enough, or do we need something more granular

Per-application granularity for temporary flags is enough. Granularity for something permanently configurable should be lower, at least because of common code principles ("no globals"), and because of different use cases in different part of the application.

Ideally it should be not just some property, but just an ability to achieve the desired behavior. In our example - the previous behavior (without clearing focus) was more configurable, because users that want to clear focus could just add a focusable background. And users that don't want to clear focus - just keep the default behavior. With the new feature, users that don't want to clear focus have no option.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Per-application granularity for temporary flags is enough

Personally I would create a common class that we can pass to every entry point, it looks better design in my eyes, but it is not so important. More important is to keep the established AOSP design for temporary flags.

@m-sasha m-sasha requested a review from igordmn November 5, 2025 18:13
@m-sasha m-sasha force-pushed the m-sasha/click-to-clear-focus branch from 7f64316 to a5cca00 Compare November 5, 2025 18:44
@m-sasha m-sasha force-pushed the m-sasha/click-to-clear-focus branch 2 times, most recently from 3cf937f to 3ee1038 Compare November 5, 2025 20:14
@m-sasha m-sasha requested a review from igordmn November 6, 2025 09:29
@m-sasha
Copy link
Author

m-sasha commented Nov 17, 2025

LGTM, but blocking until we create 1.10 branch

Is it created? Can we merge this?

@m-sasha m-sasha force-pushed the m-sasha/click-to-clear-focus branch from 3ee1038 to 43537cc Compare November 17, 2025 12:50
@igordmn
Copy link
Collaborator

igordmn commented Nov 17, 2025

Is it created?

Not yet

Copy link
Collaborator

@igordmn igordmn left a comment

Choose a reason for hiding this comment

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

LGTM for merging after the branch creation

@m-sasha m-sasha merged commit 3c5b1cf into jb-main Nov 24, 2025
18 of 19 checks passed
@m-sasha m-sasha deleted the m-sasha/click-to-clear-focus branch November 24, 2025 12:15
@mahramane
Copy link

Hi
In ExposedDropdownMenu, item selection is not possible because clicking a menu item clears the TextField focus, causing the dropdown to close immediately.

@igordmn
Copy link
Collaborator

igordmn commented Dec 17, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants