Pivot
  1. Pivot
  2. PIVOT-677

Add ability to navigate a ButtonGroup of RadioButtons using the keyboard

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.1
    • Component/s: wtk
    • Labels:
      None
    • Environment:
      n/a

      Description

      LEFT or UP arrows would select the previous enabled button in the group.
      RIGHT or DOWN arrows would select the next enabled button in the group.
      Optionally, HOME & END would select the first or last enabled button in respectively.

      This functionality could be added to the existing org.apache.pivot.wtk.ButtonGroup class, or a new class named RadioButtonGroup could be created.
      The ButtonGroup would track the order that the buttons were added and use it to select the appropriate button.

        Activity

        Hide
        Sandro Martini added a comment -

        Hi, why not fix this in the 2.0 ?

        Keyboard navigation for me (and for many of us) is very important ...

        Bye,
        Sandro

        Show
        Sandro Martini added a comment - Hi, why not fix this in the 2.0 ? Keyboard navigation for me (and for many of us) is very important ... Bye, Sandro
        Hide
        Greg Brown added a comment -

        At this point, we should only be fixing critical issues for 2.0. Users can still use the Tab key to navigate between radio buttons. This would simply be an additional convenience and can easily be resolved in a point release.

        Show
        Greg Brown added a comment - At this point, we should only be fixing critical issues for 2.0. Users can still use the Tab key to navigate between radio buttons. This would simply be an additional convenience and can easily be resolved in a point release.
        Hide
        Chris Bartlett added a comment -

        Added a org.apache.pivot.wtk.RadioButtonGroup class to provide ketboard navigation with UP, DOWN, LEFT, RIGHT, HOME & END keys.
        Simple keypresses are used to jump to the next/previous button whose rendered 'button' data begins with that character.
        Focus transfer via the TAB and SHIFT+TAB keystrokes can be modified to transfer focus out of the ButtonGroup.

        Show
        Chris Bartlett added a comment - Added a org.apache.pivot.wtk.RadioButtonGroup class to provide ketboard navigation with UP, DOWN, LEFT, RIGHT, HOME & END keys. Simple keypresses are used to jump to the next/previous button whose rendered 'button' data begins with that character. Focus transfer via the TAB and SHIFT+TAB keystrokes can be modified to transfer focus out of the ButtonGroup.
        Hide
        Greg Brown added a comment -

        I think this could produce some strange UI behavior when the buttons are not all children of the same container. Might this be more simply solved by just adding UP/DOWN/LEFT/RIGHT support to RadioButton and having it look to its parent container for other components to focus?

        Show
        Greg Brown added a comment - I think this could produce some strange UI behavior when the buttons are not all children of the same container. Might this be more simply solved by just adding UP/DOWN/LEFT/RIGHT support to RadioButton and having it look to its parent container for other components to focus?
        Hide
        Chris Bartlett added a comment -

        RadioButtonGroup extends the unchanged ButtonGroup, so the new functionality will only be exposed if the new class is specifically used. It is intended for what I consider to be the common use case where all radio buttons are in the same container, however I tried to make it behave in a predictable and sane manner for other scenarios too. The problem was not knowing what those scenarios might be.

        Greg -
        Can you give any examples of having buttons in different containers where you think the behaviour might be strange? And are you talking about the focus side of things or navigation/selection too?

        The only multi-container scenario I could think of for the demo was having RadioButtons aligned via a TablePane's columns, and that looks fine to me. See the RadioButtons in 'Container 1' to 'Container 6' in the demo app, which can be found here
        http://svn.apache.org/repos/asf/pivot/trunk/examples/src/org/apache/pivot/examples/buttons/radio_button_group_example.bxml

        I'm not clear about what you are suggesting with RadioButton (And again, are you referring to just focus or selection/navigation?).
        Could you elaborate a bit further?
        Sorry if I am being slow

        Show
        Chris Bartlett added a comment - RadioButtonGroup extends the unchanged ButtonGroup, so the new functionality will only be exposed if the new class is specifically used. It is intended for what I consider to be the common use case where all radio buttons are in the same container, however I tried to make it behave in a predictable and sane manner for other scenarios too. The problem was not knowing what those scenarios might be. Greg - Can you give any examples of having buttons in different containers where you think the behaviour might be strange? And are you talking about the focus side of things or navigation/selection too? The only multi-container scenario I could think of for the demo was having RadioButtons aligned via a TablePane's columns, and that looks fine to me. See the RadioButtons in 'Container 1' to 'Container 6' in the demo app, which can be found here http://svn.apache.org/repos/asf/pivot/trunk/examples/src/org/apache/pivot/examples/buttons/radio_button_group_example.bxml I'm not clear about what you are suggesting with RadioButton (And again, are you referring to just focus or selection/navigation?). Could you elaborate a bit further? Sorry if I am being slow
        Hide
        Greg Brown added a comment -

        Sorry if I wasn't clear. It just seems like you might be able to save yourself some code by taking advantage of the fact that a Container is already a sequence of components. Instead of extending ButtonGroup to behave like a sequence, just have radio buttons use the parent container to determine what to focus when an arrow key is pressed (e.g. have the up arrow call transferFocus(FocusTraversalDirection.PREVIOUS)).

        Show
        Greg Brown added a comment - Sorry if I wasn't clear. It just seems like you might be able to save yourself some code by taking advantage of the fact that a Container is already a sequence of components. Instead of extending ButtonGroup to behave like a sequence, just have radio buttons use the parent container to determine what to focus when an arrow key is pressed (e.g. have the up arrow call transferFocus(FocusTraversalDirection.PREVIOUS)).
        Hide
        Chris Bartlett added a comment -

        I'd like to clarify too. The keyboard 'navigation' as I have referred to it will both select and focus a button, not just transfer focus while awaiting selection via a mouse click or SPACE key press. Focus can still be transferred with the TAB or SHIFT+TAB keys as usual (when the 'intraGroupFocusTransferEnabled' property is set).

        This solution delegates to Component#transferFocus(FocusTraversalDirection) when the 'intraGroupFocusTransferEnabled' property is false. It is used to determine how focus should be transferred away from the group, treating the group as a single entity for focusability via the TAB/SHIFT+TAB key presses.

        Maintaining a single List of Buttons is not much of a chore, and serves to keep all the logic in one place. I did consider changing ButtonGroup's internal collection to a List (enforcing uniqueness) from a Group so that RadioButtonGroup could reuse it too. However the few LOC that might be saved didn't seem to warrant it, and it didn't smell right!

        The 'Container 1' to 'Container 6' scenario (from the previously mentioned demo app) uses a RadioButtonGroup to allow RadioButtons to behave as a group (with keyboard navigation) even each buttons is in a different container. I can't see how Component#transferFocus would help in this case, but I don't think you were considering such a scenario.

        Show
        Chris Bartlett added a comment - I'd like to clarify too. The keyboard 'navigation' as I have referred to it will both select and focus a button, not just transfer focus while awaiting selection via a mouse click or SPACE key press. Focus can still be transferred with the TAB or SHIFT+TAB keys as usual (when the 'intraGroupFocusTransferEnabled' property is set). This solution delegates to Component#transferFocus(FocusTraversalDirection) when the 'intraGroupFocusTransferEnabled' property is false. It is used to determine how focus should be transferred away from the group, treating the group as a single entity for focusability via the TAB/SHIFT+TAB key presses. Maintaining a single List of Buttons is not much of a chore, and serves to keep all the logic in one place. I did consider changing ButtonGroup's internal collection to a List (enforcing uniqueness) from a Group so that RadioButtonGroup could reuse it too. However the few LOC that might be saved didn't seem to warrant it, and it didn't smell right! The 'Container 1' to 'Container 6' scenario (from the previously mentioned demo app) uses a RadioButtonGroup to allow RadioButtons to behave as a group (with keyboard navigation) even each buttons is in a different container. I can't see how Component#transferFocus would help in this case, but I don't think you were considering such a scenario.
        Hide
        Greg Brown added a comment -

        OK, that helps a bit. I see that simply transferring focus wouldn't be sufficient - you'd also need to know that you are transferring focus to a button and that the button would need to be selected, which isn't quite as elegant.

        It still seems odd to me that RadioButtonGroup doesn't extend Container, though. In most cases, when you want this kind of behavior, I'd guess that all of the buttons will belong to a single parent container (most likely a BoxPane). In that case, it seems like either extending Container or adding a simple key listener to the container would be sufficient (and potentially simpler).

        Show
        Greg Brown added a comment - OK, that helps a bit. I see that simply transferring focus wouldn't be sufficient - you'd also need to know that you are transferring focus to a button and that the button would need to be selected, which isn't quite as elegant. It still seems odd to me that RadioButtonGroup doesn't extend Container, though. In most cases, when you want this kind of behavior, I'd guess that all of the buttons will belong to a single parent container (most likely a BoxPane). In that case, it seems like either extending Container or adding a simple key listener to the container would be sufficient (and potentially simpler).
        Hide
        Greg Brown added a comment -

        OTOH, perhaps a "selectOnFocus" style for RadioButton would be appropriate. That way, transferring focus would be sufficient.

        Not sure what other implications it might have - just thinking out loud.

        Show
        Greg Brown added a comment - OTOH, perhaps a "selectOnFocus" style for RadioButton would be appropriate. That way, transferring focus would be sufficient. Not sure what other implications it might have - just thinking out loud.
        Hide
        Chris Bartlett added a comment -

        I simply 'enhanced' ButtonGroup. If that had extended Container, then RadioButtonGroup obviously would have used this to its advantage as you suggest.

        I agree that the common scenario is to have all buttons within a single parent Container, but again, this was not true of ButtonGroup, although I thought it would be. After thinking it over I came up with the 'RadioButtons in a TablePane' scenario, which I think is a valid and can imagine being used in a real layout. This solution allows that but I don't think a traditional Container would.

        Show
        Chris Bartlett added a comment - I simply 'enhanced' ButtonGroup. If that had extended Container, then RadioButtonGroup obviously would have used this to its advantage as you suggest. I agree that the common scenario is to have all buttons within a single parent Container, but again, this was not true of ButtonGroup, although I thought it would be. After thinking it over I came up with the 'RadioButtons in a TablePane' scenario, which I think is a valid and can imagine being used in a real layout. This solution allows that but I don't think a traditional Container would.
        Hide
        Greg Brown added a comment -

        ButtonGroup was designed to accommodate both scenarios - where all buttons have the same parent as well as different parents. The purpose of a button group is simply to enforce that only a single button can be selected at a time. The idea was that any navigation would be the responsibility of the buttons' containers.

        By defining two sequences of buttons (RadioButtonGroup as well as the parent container), we create a situation where traversal order could get out of sync (if I define the buttons in one order in the RadioButtonGroup but another order in the container). I understand that it isn't likely, but could generate confusion. It also requires the caller to explicitly define the sequence twice - once in the container and once in the group.

        So what I'm saying is that I don't personally think extending ButtonGroup is the right solution to this problem. FWIW, in the case where the buttons do not share a parent, they will most likely share some common ancestor - the key listener could perhaps be attached to that container instead.

        Show
        Greg Brown added a comment - ButtonGroup was designed to accommodate both scenarios - where all buttons have the same parent as well as different parents. The purpose of a button group is simply to enforce that only a single button can be selected at a time. The idea was that any navigation would be the responsibility of the buttons' containers. By defining two sequences of buttons (RadioButtonGroup as well as the parent container), we create a situation where traversal order could get out of sync (if I define the buttons in one order in the RadioButtonGroup but another order in the container). I understand that it isn't likely, but could generate confusion. It also requires the caller to explicitly define the sequence twice - once in the container and once in the group. So what I'm saying is that I don't personally think extending ButtonGroup is the right solution to this problem. FWIW, in the case where the buttons do not share a parent, they will most likely share some common ancestor - the key listener could perhaps be attached to that container instead.
        Hide
        Chris Bartlett added a comment -

        It's a shame you didn't have time to add some comments to this ticket before I proceeded with this. It would have saved me a lot of time.

        I don't think there is much to add about the ordering side of things. Any 'confusion' generated should be mild and have a fairly obvious solution.

        I can think of a number of potential workarounds/redesigns that I think would address the points you raise, but I get the feeling that none would be satisfactory if the solution remains an extension of ButtonGroup.

        Show
        Chris Bartlett added a comment - It's a shame you didn't have time to add some comments to this ticket before I proceeded with this. It would have saved me a lot of time. I don't think there is much to add about the ordering side of things. Any 'confusion' generated should be mild and have a fairly obvious solution. I can think of a number of potential workarounds/redesigns that I think would address the points you raise, but I get the feeling that none would be satisfactory if the solution remains an extension of ButtonGroup.
        Hide
        Greg Brown added a comment -

        Sorry - I didn't see the ticket until I reviewed the commit message. This may be a good example of a situation where a new feature should be proposed and discussed on the list before filing a ticket.

        Show
        Greg Brown added a comment - Sorry - I didn't see the ticket until I reviewed the commit message. This may be a good example of a situation where a new feature should be proposed and discussed on the list before filing a ticket.
        Hide
        Chris Bartlett added a comment -

        Fair enough, although you did comment on the ticket back in December when it was created.

        What I have written meets my needs, so I will continue to use it, but someone else may be willing to have another crack at a it.

        Would you prefer I back this out?

        Show
        Chris Bartlett added a comment - Fair enough, although you did comment on the ticket back in December when it was created. What I have written meets my needs, so I will continue to use it, but someone else may be willing to have another crack at a it. Would you prefer I back this out?
        Hide
        Greg Brown added a comment -

        Oh yeah, so I did see it. OTOH, there wasn't any additional discussion on it until today.

        It's up to you whether to leave it in or not.

        Show
        Greg Brown added a comment - Oh yeah, so I did see it. OTOH, there wasn't any additional discussion on it until today. It's up to you whether to leave it in or not.
        Hide
        Chris Bartlett added a comment -

        No further comments, so marking as resolved.

        Show
        Chris Bartlett added a comment - No further comments, so marking as resolved.

          People

          • Assignee:
            Chris Bartlett
            Reporter:
            Chris Bartlett
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development