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

Clarify the relation of focus and propagate_to_hidden #1716

Closed
xarvic opened this issue Apr 12, 2021 · 5 comments
Closed

Clarify the relation of focus and propagate_to_hidden #1716

xarvic opened this issue Apr 12, 2021 · 5 comments
Labels
question causes uncertainty

Comments

@xarvic
Copy link
Collaborator

xarvic commented Apr 12, 2021

As i understand it, hidden in propagate_to_hidden() means the widget, is inside the widget tree, but can't be seen. Not because it is overlaid by another component, but because it doesn't have a position inside the current window (like a widget inside a hidden Tab). On the other hand, a component in a scroll would not be "hidden" according to my understanding, even if you can't see it right now.
Therefore a component can participate in focus if and only if it is not hidden.
Which means for one thing that my implementation for the Lifecycle::BuildFocusChain in #1632 is wrong since it includes all hidden tabs. But also, to fix this we would probably need a context method children_state_changed() which would request a rebuild of the focus_chain (for an example when selecting another Tab). Calling children changed should be enough.

Another thing we should think about is what happens to focused widgets which become hidden.

@cmyr cmyr added the question causes uncertainty label Apr 12, 2021
@cmyr
Copy link
Member

cmyr commented Apr 12, 2021

Your understanding is correct; a 'hidden' widget is a widget that is part of the widget tree but is not currently a functional part of the UI, generally because it is part of a switch/either/tab that is not visible.

I also agree that a widget that is in a scroll view and is not onscreen is not "hidden".

@xarvic
Copy link
Collaborator Author

xarvic commented Apr 13, 2021

Ok, then i will create a PR with the changes. I would like to test in BuildFocusChain if the focused widget is still a part of the non hidden tree, send ResignFocus otherwise. Is that ok? I also would like to change Lifecycle::should_propagate_to_hidden() -> bool to Lifecycle::propagate_to_hidden() -> Option<Self>. Then we could change the focus event into a focus event with only the resign part. Therefore even if someone tries to focus a hidden widget he won't succeed, but were still able send the unfocus to the specific widget.

@cmyr
Copy link
Member

cmyr commented Apr 13, 2021

Ok, then i will create a PR with the changes. I would like to test in BuildFocusChain if the focused widget is still a part of the non hidden tree, send ResignFocus otherwise. Is that ok? I also would like to change Lifecycle::should_propagate_to_hidden() -> bool to Lifecycle::propagate_to_hidden() -> Option<Self>. Then we could change the focus event into a focus event with only the resign part. Therefore even if someone tries to focus a hidden widget he won't succeed, but were still able send the unfocus to the specific widget.

The first part sounds good. The second part is... maybe okay? I'm always a little scared of complicating the logic more than possible. Thinking this through, I'm not sure if it really matters if a hidden widget gets the 'resign' message, assuming it gets some message when it becomes hidden? (does this happen? 😬)

@xarvic
Copy link
Collaborator Author

xarvic commented Apr 13, 2021

I don't think so. Since hidden means the parent widget decided at some point not to show the widget anymore, the widget itself doesn't know that it's hidden. If a textbox inside a tab widget is focused and you select another tab, druid thinks the textbox is still focused until you click another one.

@xarvic
Copy link
Collaborator Author

xarvic commented May 16, 2021

I think we can close this now

@xarvic xarvic closed this as completed May 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question causes uncertainty
Projects
None yet
Development

No branches or pull requests

2 participants