Page MenuHomePhabricator

focus: do not allow focus to exit a popup
ClosedPublic

Authored by segfaultxavi on Oct 17 2018, 7:28 AM.

Details

Summary

Popup windows dim the background and do not allow the user
to click outside the popup. However, pressing TAB can take
you to widgets on the background. This didn't happen in
legacy.

Fixes T7432

Test Plan

Instructions in T7432

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
segfaultxavi created this revision.Oct 17 2018, 7:28 AM
segfaultxavi requested review of this revision.Oct 17 2018, 7:28 AM

From the focus POV - this looks completly right :)

@Jaehyun_Cho Is it okay from the widget POV ? :)

@bu5hm4n @segfaultxavi

From widget's behavior POV, it is fine. But I think we need to resolve multiple class inheritance issue in C# beforehand.

mixin Efl.Ui.Focus.Layer inherits from Efl.Ui.Widget so it requires multiple class inheritance in C#.
Therefore, I think this multiple class inheritance issue should be resolved before applying this patch.

Please refer the following comment on T7366.

@lauromoura @felipealmeida @bu5hm4n

In the latest devs/lauromoura/csharp-new-classes branch, all the duplicated re-implemented methods are removed. (parent's methods are used instead)
e.g. Efl.Ui.Widget.FocusStateApply() is not re-implemented in Efl.Ui.Layout.Object because efl_ui_layout_object.eo does not implement focus_state_apply.

However, it reveals mixin's hierarchy problem because eo mixin is converted to EFL# interface so eo mixin's abstract class parent is removed in EFL# interface's parent list.
e.g. mixin Efl.Ui.Focus.Layer does not inherit from Efl.Ui.Widget in EFL#. So Efl.Ui.Focus.Layer's re-implemented FocusStateApply() cannot be called although efl_ui_focus_layer.eo implements focus_state_apply.

I think we need to think how to resolve this mixin case here again for language bindings.

Shouldn't it be:
class Efl.Ui.Popup(Efl.Ui.Widget, Efl.Content, Efl.Ui.Focus.Layer) ?

This way, there isn't multiple inheritance.

If Efl.Ui.Popup inherits from Efl.Ui.Widget instead of Efl.Ui.Layout, the popup is not rendered correctly: The button's center is in one corner of the window, so only one quarter of the button is visible, and there's no trace of the popup window.

At the risk of repeating myself, what is exactly the problem with mixins in C#? This patch seems to work perfectly fine (once rebased), even with the multiple inheritance produced by Efl.Ui.Layout and Efl.Ui.Focus.Layer both inheriting the abstract class Efl.Ui.Widget.

Rebased after Efl.Ui.Layout.Object was renamed to Efl.Ui.Layout

zmike accepted this revision.Jan 28 2019, 9:35 AM
This revision is now accepted and ready to land.Jan 28 2019, 9:35 AM
This revision was automatically updated to reflect the committed changes.
zmike added a comment.Jan 28 2019, 9:38 AM

I manually rebased this before landing.