[patch] enable TAB to quickly toggle dialogsPanel

[cmuelle8]

The patch enables JOSM users to quickly toggle the dialogsPanel on the right (and toolbar/menubar). This is helpful especially on small screens used by e.g. netbooks.

This is comparable to what GIMP does to toogle docked dialogs in single window mode.

​http://www.gimp.org/release-notes/gimp-2.8.html

Greetings

[cmuelle8]

JOSM-toogleDialogsPanel-with-tabulator-key.patch

[stoecker]

JOSM uses keyboard shortcut assignment. Please use the appropriate classes, otherwise shortcut handling is broken. Hardcoded keys are unwanted.

[cmuelle8]

Replying to stoecker:

JOSM uses keyboard shortcut assignment. Please use the appropriate classes, otherwise shortcut handling is broken. Hardcoded keys are unwanted.

It is not hardcoded and does register the shortcut just as fullscreen toggling does, which uses F11 by default, but is (also) reconfigurable iirc. The first attachment was missing a (new) file in the patch, maybe this interfered with your comment.

Greetings

[stoecker]

The "free TAB key" part is hardcoded.

TAB is currently taken by Draw action - DevelopersGuide/ShortcutsList.

There should be no conflicts in core.

[cmuelle8]

Replying to stoecker:

The "free TAB key" part is hardcoded.

TAB is currently taken by Draw action - DevelopersGuide/ShortcutsList.

There should be no conflicts in core.

Thanks for the pinpoint, this more or less makes the patch useless, since the patch intended to mimic GIMPs behavior. Unless a consent upon reassigning the default shortcut for angle snapping may be found that is. TAB usage for angle snapping might be a semi-standard and stem from another software though.

Deregistering TAB could be done conditionally, depending on wheter TAB is assigned to /this toggle action/ in JOSMs global shortcut list. Since toggling fullscreen or the visibility of dialogsPanel often moves the current focus to the VerticalToolbar on the left, it'd be a pain for smooth toggling if TAB is not deregistered with the FocusManager. In this case the action cannot be reversed smoothly by repeatedly pressing TAB.

options:

• find another default key for the action (and refrain from making this compatible with GIMPs behavior)
• reassign angle snapping default key to an alternative

Would angle snapping benefit from a freed TAB key on toolbars as much as dialogs toggle action would? If so, the condition for deregistering TAB boils down to determine if TAB is found in the shortcut list, regardless of the action assigned to it.

Requesting for comments, basically..

[akks]

Tab in angle snap action was not very good idea, for a long time it is duplicated by the repeated A action and can be easily turned off (at least, by default).

The question is - would the patch work for those who use TAB for navigation 'inside' toggle dialogs.

[stoecker]

If TAB for angle-snap is anyway duplicate, I think we can remove it there. We anyway have shortcut-shortage, so we don't need to waste any. We can assign an unused code as default, so user can re-add a shortcut.

[cmuelle8]

Replying to akks:

The question is - would the patch work for those who use TAB for navigation 'inside' toggle dialogs.

TAB inside toggle dialogs works, I've tested it inside layer dialog and tagging dialog table. TAB on title bars of toggle dialogs does not, i.e. you can't use it to walk the focus cycle on toggle-dynamic/undock/close buttons located right on the title bar. The dynamic buttons themselves might be affected as well, but a table or list inside a toggle dialog should be walkable with TAB once any element was focused using the mouse.

[akks]

I have tested patch and now I am ready to commit it (and turn off Tab for angle snapping), but there are few questions:

• Is Tab button really suitable? It seems to be unused outside toggle dialogs, but maybe I miss something? Maybe some Linux windows managers
• I am not sure hiding the top toolbar is a very good idea. This could be optional (I'll add it if needed into tollbar context menu). What about main menu?

[akks]

I have added checkbox "Do not hide toolbar and menu" and included AngleSnapping tab turn-off. Please check when the patch is ready (fixing last problem)

[cmuelle8]

Thx for additional feature, maybe we should make toggling of main window elements configurable as an expert option all together, allowing to choose a custom set out of

menu bar
top toolbar
left side bar
status bar
toggle dialogs

this way users can decide themselves what's important for them and you do not waste extra space in menu, as I understand is currently taken to decide if menu bar/top toolbar is hidden or not.

As for your concern with Unix WMs, I run a copy on Linux, KDE, xfce, fluxbox are all fine. And GIMP, where the idea stems from, does it the same way.

Thx for your efforts.

[akks]

Here is the updated patch. The additions:

• remove default Tab button for angle snapping
• added checkbox in "Do not hide toolbar and menu" in toolbar context menu
• de-register Tab in MapFrame only if needed
• rework context menus for toolbar (one menu for all buttons now, is changing dynamically)

I am not sure about options to turn off left side bar and status bar... Left sidebar is already hideable. Can add it if needed.

[akks]

JAR for testing (would be the best if the jar was created automatically on server for all patches and commits...):
​https://dl.dropboxusercontent.com/u/63393258/josm-custom.jar

[cmuelle8]

I'll tested your patch and it works well. I found a single show stopper: Double click in tagging dialog opens a tag edit dialog. Within this dialog usual behavior of TAB is needed. This works for the relation dialog however, I suspect this is due to tag edit and relation edit dialogs having different parents. Also, the former is modal, the latter not.

We either need to specifically enable focus traversal keys on the dialog before it is being shown, or we need to change it's parent (but stand-alone may not work as I understand it needs to stay modal).

Greetings

[cmuelle8]

I patched ExtendedDialog, superclass of TagEditHelper to circumvent this with two additional lines. I'm looking to reuse your way of "donothide" for toolbar+menu for context menus to left sidebar and statusbar. Will post revised patch then.

[cmuelle8]

• statusbar context menu added, similar to toolbar
• ExtendedDialog patched to traverse Components with TAB (is a no-op if TAB is not assigned to any action, hence run unconditionally)

[akks]

Tried to apply the patch. Tab does not work in Alt-A if Tab is assigned to some other action, not Toggle dialog panel. Maybe I introduced this bug myself...
Maybe we should do deregistering Tab only for this action, not for any of them?

[cmuelle8]

Replying to akks:

Tried to apply the patch. Tab does not work in Alt-A if Tab is assigned to some other action, not Toggle dialog panel. Maybe I introduced this bug myself...
Maybe we should do deregistering Tab only for this action, not for any of them?

No, this should work for any action. And if it did not work before, then yes, it might be a different bug. But it's well in scooe here so we can fix it all together. I'll try to reproduce your report and spot the remaining issue.

Also, "do not hide" in PopupMenu for side toolbar is still missing, but easy to add, I found the spot to put it in, it already has a menu for removing buttons.

Greetings

[cmuelle8]

Mornin,

I did some housekeeping in MapFrame along the way. I added "Do not hide" style popup menu entries to side toolbar and status line.

Good(?) news: I could not reproduce "TAB not working in Alt-A". It worked as expected, restoring traversal defaults in ExtendedDialogs, for the following test cases / modifications to josm's global shortcuts:

• TAB assigned to toggle dialogs action
• TAB unassigned
• TAB assigned to "move map upward" action
• TAB assigned to "Select Map Mode" action

If the changes to ExtendedDialog do not work for you, then my guesses for trouble shooting are

• system dependency, what system are you on? (i tested winxp and linux)
• you have a different DefaultFocusManager that does not include TAB in defaultFocusTraversalKeys for FORWARD_TRAVERSAL (maybe on an "apple"?)

What is the benefit of using org.openstreetmap.josm.gui.widgets.PopupMenuLauncher? This class seems dated to me. Are there reasons to not just use Component.setComponentPopupMenu() and eventually Component.setInheritsPopupMenu() for the toolbars?

Your rewrite of top toolbar popup menu makes use of PopupMenuLauncher class. It

• implements MouseAdapter and uses isPopupTrigger() on several MouseEvents it overrides to determine if a certain mouse event is a popup trigger on a specific platform, setComponentMenu() seems to do the same automagically: "Sets the JPopupMenu for this JComponent. The UI is responsible for registering bindings and adding the necessary listeners such that the JPopupMenu will be shown at the appropriate time. When the JPopupMenu is shown depends upon the look and feel: some may show it on a mouse event, some may enable a key binding."

• tries to focus the component on launch(), but for toolbars this is not needed at all
• adds a single member aside the wrapped JPopupMenu, checkEnabled, again propably not needed in toolbar context
• superclasses TextContextualPopupMenu, so this might be its major use

I suggest to try to avoid the bloat of project code in favor of jdk functions where possible. Unless there is a good argument to use josm's PopupMenuLauncher widget class for the toolbars, should we not drop this dependency in favor of the jdk functions?

A remaining todo might be to streamline the popup menu handling of top toolbar with the one of side toolbar. I did not rework that in the same fashion as you did for the former, since I'm unsure about PopupMenuLauncher usage. If a decision on that has been made, it would be desirable to have the "dynamically adapting, button dependent" popup menus for the vertical toolbar as well, i.e. button dependent menu entries first, whole-toolbar dependent last.

Suggest to open another ticket for this todo and apply the patch to trunk, if it works for you (in particular Alt+A stuff).

Greetings

[cmuelle8]

Btw, usage of this dated PopupMenuLauncher is the single reason why context menus are not shown when the corresponding keyboard key is pressed, i.e.

​http://en.wikipedia.org/wiki/Menu_key

Registering the popup menu using JDK's setComponentPopupMenu seems to do the right thing (tm), so rethinking PopupMenuLauncher alltogether (not just for toolbars) might be a good thing.

[cmuelle8]

revised-dialogs-toggle-action.patch

[cmuelle8]

• close remaining todo suggested at the end of comment:18
• streamlines context menu handling for toolbars
• use jdk methods, drop dependency on josm widget PopupMenuLauncher
• change setActionAndAdapt from public to private member
• this uses JPopupMenu.getInvoker() to get the relevant button / source components

            addPopupMenuListener(new PopupMenuListener() {
                public void popupMenuWillBecomeVisible(PopupMenuEvent e) {
                    setActionAndAdapt(buttonActions.get(
                            ((JPopupMenu)e.getSource()).getInvoker()
                    ));

[Don-vip]

Replying to cmuelle8:

Registering the popup menu using JDK's setComponentPopupMenu seems to do the right thing (tm), so rethinking PopupMenuLauncher alltogether (not just for toolbars) might be a good thing.

Thanks for this remark, I'll look into it (wasn't aware of these jdk methods)

[cmuelle8]

Replying to Don-vip:

Replying to cmuelle8:

Registering the popup menu using JDK's setComponentPopupMenu seems to do the right thing (tm), so rethinking PopupMenuLauncher alltogether (not just for toolbars) might be a good thing.

Thanks for this remark, I'll look into it (wasn't aware of these jdk methods)

Yeah, sorry that this info didn't "hit" you at an earlier time. I just noticed you spent some time with PopupMenuLauncher on your last commit. At least for the toolbars this class does not seem to be needed at all. I have not looked at the toggle dialogs in detail to dare make the same assumption for components with table/list/trees, but /maybe/ you can check if this simplifies some code further given that you have it on mind currently.

[Don-vip]

For the simplest ones it should eliminate the need of this class. I'm not so sure about those needing to adapt dynamically the popup menu.

[anonymous]

Replying to Don-vip:

For the simplest ones it should eliminate the need of this class. I'm not so sure about those needing to adapt dynamically the popup menu.

The patch does exactly this.

Currently PopupMenu for top and side toolbar is adapted dynamically. Strg+F the patch for

addPopupMenuListener

you will find two ways to do it, one within a named (ToolbarPreferences.java), the other within an anonymous subclass of JPopupMenu (MapFrame.java). They both do dynamic updating, without the launcher widget.

Important part is getInvoker(). However, in toolbars there is no need to do coordinate resolution to get a certain table cell, just getting the component suffices. Maybe it's harder to get to a table cell given only a PopupMenuListenerEvent.

Regards,
cmuelle8

[Don-vip]

Ah nice. Didn't look at the patch yet but looks promising !

[akks]

@cmuelle8 : thank you for the patch very much!
New approach to context menus looks really good.
I'll commit it now, separated in two parts:

1. Context menu rework and toggle action.
2. Shortcut and focus related realted changes.

[akks]

In 5965/josm:

see #8652 (patch by cmuelle8, part 1, modified): add "toggle panels" action (soon will be on Tab)
rework all toolbar context menus (add "do not hide" item and remove PopupMenuLauncher usage)
fixing minor warnings (@Override, private final)

[akks]

the rest of the patch, contain Tab-related changes

[akks]

I pathched the status bar menu code when committinig (there were conflicts with AngleSnap menu and JumpToAction).

The Tab-related changes need some rework. Currently Tab should be enabled in all dialogs - it is done in ExtendedDialog, but there are many others (like "rename layer" and Yes/No/Cancel messageboxes). They do not use Josm code, so there are no easy way to restore its old Tab behavior.

Maybe we should not de-register Tab globally but do something else instead... Can someone give and idea?

[Don-vip]

Plugin ElevationProfile must be updated after r5965 changes.

[Don-vip]

Replying to Don-vip:

Plugin ElevationProfile must be updated after r5965 changes.

Done in [o29611].

[akks]

It seems I have found a solution for Tab-problem:
setFocusTraversalKeysEnabled(false) in MapView constructor instead of fixing KeyboardFocusManager.

Drawback: toggling dialogs only works when main map is focused. But it even seems logical )

[akks]

In 5979/josm:

fix #8652 [based on patch by cmuelle8] enable dialog and panels hiding by TAB
disable TAB for angle snapping by default (please use A-A or assign different key)

[akks]

I feel really sorry about commiting in stabilization phase again, but the feature seems to be long-awaited (and tab for angle snapping - too deprecated to keep it for another month). I have also fixed one mistake in context menu ("Do not hide toolbar and menu" instead of "Hide toolbar and menu").

I checked this for different look and feels and it seems to work.
Please revert shortcut-related changes in case of some problems that can not be solved before tested is released.

[cmuelle8]

Thanks for putting this in, but current scope of Tabulator key is not enough. E.g. there are situation where we do not have a MapView at all, but need to toggle into or back out of dialogspanel-hidden-mode. I.e. currently, if there is just GPX or WMS / TMS layers we cannot use Tab, even if those layers are focused.

The worst situation you can get into is hiding dialogspanel+menu without a current MapView - then Tab will not be to the rescue to show menu again. We save this in preferences so the program's state will persist across restarts..

I did not look into a solution yet, just the report for now. Feel free to open a new ticket if need be.

[akks]

Thank you for report, I'll try to fix it now.

[akks]

The most strange thing is that MapView is created for opened GPX, and setFocusTraversalKeysEnabled(false); is called successfully. But the Tab key does not work (actionPerformed is not called, I checked). If one adds OSM layer, Tab begins to work. If you remove OSM layer, Tab continues to work... Any other key for toggling works OK even with GPX layer.

[akks]

The problem was that MapView was not focused when it contain no OsmDataLayer. I am committing the fix, please check it.

[akks]

In 6018/josm:

see #8652: fix toggling panels by Tab (did not work when GPX or imagery layer was the only one)

[akks]

(one needs to click on MapView before Tab will start to work)

[anonymous]

Replying to akks:

(one needs to click on MapView before Tab will start to work)

thanks for the quick fix. now the only thing left to do, seems to be redirection of the keyboard input to the global shortcut table for /some/ gui elements, i.e. toolbars.

This differs from the first attempt of catching /only/ Tab /everywhere/ (which we scrap'd).

iirc, first item of left toolbar gets the focus when MapView is built and when items toggle from hidden to shown after Tab.

i've seen some code that does such redirection already for some of the gui's elements. if we want to leave the dialogs panel at the right alone as to not mess up any shortcuts there, we could have this input redirection for the toolbars.

this would more seamlessly integrate the feature in some cases (and we know exactly where things break - opposed to what i did initially, introducing the shortcut globally).

Greetings,
Christian

[akks]

In 6019/josm:

see #8652: do not shift the map on screen when toggling panels by Tab [idea by andygol]

[akks]

Replying to anonymous:

now the only thing left to do, seems to be redirection of the keyboard input to the global shortcut table for /some/ gui elements, i.e. toolbars.

Do you mean disabling standard-Tab for them too? Will not setFocusTraversalKeysEnabled(false) on all needed components be enough?
We should be careful not to break Tab in places where people use it. I am not sure about toggle dialogs etc.

[cmuelle8]

Replying to akks:

Replying to anonymous:

now the only thing left to do, seems to be redirection of the keyboard input to the global shortcut table for /some/ gui elements, i.e. toolbars.

Do you mean disabling standard-Tab for them too? Will not setFocusTraversalKeysEnabled(false) on all needed components be enough?
We should be careful not to break Tab in places where people use it. I am not sure about toggle dialogs etc.

Yes, you are right, this should be enough - the rest of keyboard input already gets to the shortcut handler..

So, running setFocusTraversalKeysEnabled(false) just for the toolbars should do. E.g. currently, if you open JOSM and do "New layer" focus is on the first item of the toolbar on the left. In this state Tab does not work (instead cycles the icons of the toolbar). It would be nice if behavior would equal that of MapView-focused-state.

Again, for any of the toggle dialogs on the right I'd not do setFocusTraversalKeysEnabled(false) - especially in Relation Dialogs where lists grow long some people might have a use of Tab in its default incarnation. However, I'd think that virtually noone uses Tab to cycle icons on toolbars - they are clicked or "touched".

Thx.

[akks]

In 6020/josm:

see #8652: use Tab for toggling panels even if focus is on toolbar buttons or scaling slider

[cmuelle8]

gracias, works for me now as originally intended.

[akks]

Replying to cmuelle8:

gracias, works for me now as originally intended.

Thank you for the patch and ideas!