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

masonry: rename has_focus to subtree_has_focus #617

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

Conversation

tomcur
Copy link
Member

@tomcur tomcur commented Sep 27, 2024

Implements #572 (comment)

Copy link
Contributor

@Philipp-M Philipp-M left a comment

Choose a reason for hiding this comment

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

Hmm I mean I don't have a strong opinion here, but subtree_has_focus suggests to me that only descendants of this widget may have focus. Maybe tree_has_focus? But I think just has_focus is fine too and shorter (and doesn't suggest that the global tree may have focus).

@tomcur
Copy link
Member Author

tomcur commented Oct 2, 2024

I see your point. The motivation for changing the name to something else is that other methods such as is_hot and has_pointer_capture describe the state of this widget, and not its ancestors/descendants. The current name also is quite similar to is_focused (whether this widget is focused, and not one of its descendants).

Perhaps a name like is_in_focus_path would work, though that can be confused with is_in_focus_chain

@Philipp-M
Copy link
Contributor

Philipp-M commented Oct 2, 2024

Difficult to find good/concise names that describe the difference between "only this widget" and "this widget and/or one of its descendants"...

I remember a similar naming discussion sometime back...

Maybe we should try to find a general naming solution/prefix for this kind?

I think is_ for "this widget" is and has_ for "this widget and/or one of its descendants" is ok, but as you say it's still quite similar to the one or other... contains_ is also a possibility.

Funny I just asked ChatGPT o1 preview (not that this has to say much), and it came independently (i.e. I didn't give him the hints above) to a similar conclusion, i.e. is_ for "this widget", and has_..._descendant,..._within or contains_ for "this widget and/or one of its descendants".

@tomcur
Copy link
Member Author

tomcur commented Oct 3, 2024

Maybe we should try to find a general naming solution/prefix for this kind?

That would be good. There are a number of state methods on contexts currently:

  • is_hot - this widget is the (unique) hover target of the pointer (if a widget has pointer capture, only that widget can be hot)
  • has_pointer_capture - this widget is the (unique) pointer capture target
  • is_focused - this widget is the (unique) focus target
  • has_focus - this widget or one of its descendants is focused
  • is_disabled - this widget or one of its ancestors is disabled
  • is_stashed - this widget or one of its ancestors is stashed

For disabled and stashed, there are state fields is_explicitly_{disabled,stashed} tracking the state for this specific widget. The same wording ("explicit") is used to disambiguate between request_layout (explicitly this widget) and needs_layout (this widget or any descendants has request_layout).

I think the direction of how state moves up or down the tree will usually be intuitively clear (focus bubbling to ancestors, disabling bubbling to the descendants, etc.), so IMO the wording doesn't necessarily have to disambiguate between the bubbling direction.

@xStrom
Copy link
Member

xStrom commented Oct 3, 2024

Just has_focus is a papercut because it's not immediately clear, so having a better name is a good idea.

With the current design it's always a simple path, not a branching tree, that has focus. So I would prefer path_has_focus over [sub]tree_has_focus. Alternatively is_focused_path.

Having a general solution has benefits, but we have no other use case right now so it feels YAGNI and I would delay worrying about it.

@PoignardAzur
Copy link
Contributor

PoignardAzur commented Oct 5, 2024

I guess subtree_has is a bit of a mouthful. It's annoying to find a concise term that expresses "either X or a child of X has property Y".

I don't really like the xxx_path or has_xxx_descendant solutions, I think they communicate the wrong idea.

We could reify the is_x / has_x dichotomy, in which case has_pointer_capture could be renamed to is_capture_target or something.

@tomcur
Copy link
Member Author

tomcur commented Oct 10, 2024

in which case has_pointer_capture could be renamed to is_capture_target or something

_target could (also) be reified: is_focused then becomes is_focus_target.

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.

4 participants