[jira] [Comment Edited] (FLEX-33772) Incorrect tab focus behavior (closed loops) when using focus groups (such as RadioButton components)

["Cosma Colanicchia (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FLEX-33772?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13785965#comment-13785965 ] 

Cosma Colanicchia edited comment on FLEX-33772 at 10/4/13 7:28 AM:
-------------------------------------------------------------------

Sorry about the whitespaces, I tried to conform to the indentation style found in the file itself, but probably Eclipse was messing up the original indentation.

Talking about behavior, I'd start figuring out the current one, that AFAICT looks like:
1) find the current focused item in the focusable list
2) move forward/backward to the next enabled/tabFocusEnabled entry in the list
3) if it is not part of a focus group, we're done
4) otherwise, if it is part of a group:
     4a) if it is selected, we are done
     4b) if it is not selected, cycle (0 to length-1) the entire focusable list
         and stop when a selected enabled/tabFocusEnabled entry is found belonging 
         to the group

Note that the search is always made from 0 to lenght - 1, meaning that now it is already possible to obtain different sequences moving backward and forward (in fact, I remember being able to escape some of those "closed loops" walking backward).

This is the proposed behaviour, implemented in the patch:
1) (same as current)
2) (same as current)
3) (same as current)
4) (same as current)
     4a) (same as current)
     4b) if it is not selected, cycle from the current entry index + 1 up to index - 1
         (or the inverse, in case of backward tabbing), wrapping around the array
         boundaries, and stop when:
         - a selected enabled/tabFocusEnabled entry is found belonging to the group
           (normalizing the "no selection in group" to be managed like "first element
           of group selected" case)
         - an enabled/tabFocusEnabled entry is found not belonging to the group
         
In my opinion, it should give the same results of the current implementation in case of contiguous groups (if this is verified to be true, I'd choose a single code path approach), but it improves the behavior on non-contiguous group (that is totally broken in current code, IMO) by making sure that all elements are reachable.

I will upload the test app recompiled with the monkey patch applied, to help verify the results - if you find that the behavior changed with contiguous group, that its probably unintended.

I tried to follow your sample cases, but I cannot exactly understand what you mean with "tabbing into" the group - for example, tabbing out from an interleaving non-group element (e.g. tab next from TextInput1 in your example) should be managed as "tabbing into" the group? This is the reason for the alternative approach I implemented: do not talk about "tab into" and "tab out" (which are difficult to define when groups are not contiguous), but simply make sure that the tab is never brought back (respecting the tab direction) and that it is able to reach all components, even in positioned between group elements.

Note that, among edge cases, we must also consider the case in which the first and/or the last element of the focusable lists are part of a group (I think that the proposed approach is handling this correctly).



was (Author: cosmacol):
Sorry about the whitespaces, I tried to conform to the indentation style found in the file itself, but probably Eclipse was messing up the original indentation.

Talking about behavior, I'd start figuring out the current one, that AFAICT looks like:
1) find the current focused item in the focusable list
2) move forward/backward to the next enabled/tabFocusEnabled entry in the list
3) if it is not part of a focus group, we're done
4) otherwise, if it is part of a group:
     4a) if it is selected, we are done
     4b) if it is not selected, cycle (0 to length-1) the entire focusable list
         and stop when a selected enabled/tabFocusEnabled entry is found belonging 
         to the group

Note that the search is always made from 0 to lenght - 1, meaning that now it is already possible to obtain different sequences moving backward and forward (in fact, I remember being able to escape some of those "closed loops" walking backward).

This is the proposed behaviour, implemented in the patch:
1) (same as current)
2) (same as current)
3) (same as current)
4) (same as current)
     4a) (same as current)
     4b) if it is not selected, cycle from the current entry index + 1 up to index - 1
         (or the inverse, in case of backward tabbing), wrapping around the array
         boundaries, and stop when:
         - a selected enabled/tabFocusEnabled entry is found belonging to the group
           (normalizing the "no selection in group" to be managed like "first element
           of group selected" case)
         - an enabled/tabFocusEnabled entry is found not belonging to the group
         
In my opinion, it should give the same results of the current implementation in case of contiguous groups (if this is verified to be true, I'd choose a single code path approach), but it improves the behavior on non-contiguous group (that is totally broken in current code, IMO) by making sure that all elements are reachable.

I will upload the test app recompiled with the monkey patch applied, to help verify the results - if you find that the behavior changed with contiguous group, that its probably unintended.

I tried to follow your sample cases, but I cannot exactly understand what you mean with "tabbing into" the group - for example, tabbing out from an interleaving non-group element (e.g. tab next from TextInput1 in your example) should be managed as "tabbing into" the group? This is the reason for the alternative approach I implemented: do not talk about "tab into" and "tab out" (which are different to define when groups are not contiguous), but simply make sure that the tab is never brought back (respecting the tab direction) and that it is able to reach all components, even in positioned between group elements.

Note that, among edge cases, we must also consider the case in which the first and/or the last element of the focusable lists are part of a group (I think that the proposed approach is handling this correctly).


> Incorrect tab focus behavior (closed loops) when using focus groups (such as RadioButton components)
> ----------------------------------------------------------------------------------------------------
>
>                 Key: FLEX-33772
>                 URL: https://issues.apache.org/jira/browse/FLEX-33772
>             Project: Apache Flex
>          Issue Type: Bug
>          Components: Focus Manager
>    Affects Versions: Apache Flex 4.11.0
>            Reporter: Cosma Colanicchia
>            Assignee: Alex Harui
>            Priority: Minor
>              Labels: patch
>         Attachments: FocusManager.patch, TestFocus.mxml, TestFocus-screenshot.png
>
>
> When additional components that are not part of the focus group are positioned between two group elements, it may be impossible to move the focus away from the group using the keyboard.



--
This message was sent by Atlassian JIRA
(v6.1#6144)