[additional PATCH] Allow hiding buttons in left toolbar

[Zverikk]

Now it is possible to hide some panel buttons, but not instruments. An experienced mapper almost always select tools not by clicking a button, but by using shortcuts, so in order to free more screen space it'd be nice to have option of hiding rarely used buttons.

[akks]

I have made a patch to add toolbar context menu ("remove button" and "show preferences" for now).

There is unsolved problem - I do not know, how to show specific tab of preferences, not the first [without big refactoring]. There is no list of preference tabs, tabs even does not have titles to use for search~~

Please have a look at it if you have some time.

[akks]

Next version of the patch also includes methods for selecting specific preferences tab (small additions to PreferenceDialog and PreferenceTabbedPane). Such method is used to open toolbar preferences directly from context menu.

[stoecker]

In [4586/josm]:

fix #6953 - patch by akks - Allow hiding tool buttons from the edit toolbar

[Zverikk]

Thanks, but edit toolbar is the one to the left, not the top one.

[akks]

Lower buttons (Tool windows) already can be hidden from its context menu. Only instruments remain and it is harder to add "hide", beause it does not have toobar editor!

Maybe we should simply make left toolbar floatable?

[stoecker]

What do you mean with "floatable"?

[akks]

toolBarActions.setFloatable(false); -> toolBarActions.setFloatable(true); in MapFrame.java and we can drag the toolbar

But some sizing and docking properties need fixing (I have no experience in complex Swing GUI).

[Zverikk]

I guess making it floatable is a bad idea. Edit toolbar is ok, it's just some buttons are more useful than others. If there is a way to hide lower buttons, why not extend it to the upper ones?

[akks]

"Extension" means ~100-strings copypasting, unfortunately :(
Current realization is for "ToggleDialog" class only, not for IconToggleButton, and there are actually TWO independent toolbars on left side - ediatble one and noneditable one:)

[akks]

Big copypaste is finished, at last - almost 2 hours. It was good for exploring Swing...
I can not call resulting code "elegant", but it works and it is slightly simpler than for second toolbar.

Here is the patch, you can rework it if needed :)

[stoecker]

Why otherMapMode? Other for the below buttons was used as the have no name. MapMode buttons have a name: MapMode :-).

Can't the code for these two buttons sets be unified? Must it really be copied? Especially with respect to long-standing request #59.

[akks]

Replying to stoecker:

Why otherMapMode? Other for the below buttons was used as the have no name. MapMode buttons have a name: MapMode :-).

Can't the code for these two buttons sets be unified? Must it really be copied? Especially with respect to long-standing request #59.

"otherMapMode" is not a good name, I agree.

Unification is rather hard, I thought of it.
Problems are:

• ToggleDialog is a class with its own functionality and additional settings. Saving preferences is implemented inside it - it is logical.
• There is no list of MapMode buttons, except toolGroup ButtonGroup contents.
• IconToggleButton is ndefinetly not a place to add preferences...
• ToggleWindow toolbar is refilled with buttons every time, while I have just used setVisible() to hide buttons. - which is better?
• Classes with menus are not easy to unify. ToggleDialog menu (OtherButtonsAction) needs access to ToggleDialog methods. We need additional adapter for IconToggleButton?

I did not notice #59. It actually contains lot of ToggleDialog refactoring and idea to split MapFrame and dialogs lificycle. Should MapMode initialization go somewhere too?

I do not know, what to do next... Maybe xeen can finish main refactoring and then I can do unified menus, if needed? Or in reverse order?

[stoecker]

I applied large parts of code from #59. Please improve your patch for recent SVN.

[anonymous]

Should I make another menu for MapModes instead of adding "ohter" button?

[akks]

!!! [currently I am reworking the patch - some variables name are not correct!!! ] !!!

Here is updated patch. It is almost the same copy-paste - style. I do not know is it suitalbe...

I moved "button_hidden" preference management to IconToggleButton (unlike ToggleDialog). I am not sure how to unify button visibilty mamagement - maybe, concentrate it in IconToggleButton? [no, ToggleDialog it seems that ToggleBDialog notify the its button is shown /hidden ] I also do not understand added "menu system" clear enough to append new menu for MapModes :)

[stoecker]

After short review: Is the "final" for addmapMode() really required? It changes the signature of a Plugin-used function requiring update of all plugins.

[akks]

!!! [currently I am reworking the patch - some variables names are not correct and leads to incorrect meaning !!! ]

final is required to show context menu... I know how to remove it, one minute please :)

[akks]

Here is a corrected version - "hidden" now means hidden, not "visible" and I also removed "final" from addmapMode() by introducing new final local variable bb.

[akks]

I have a feeling that all button-related methods should be removed from ToggleDialog. All showing/hiding/setting hidden-preferences for button should go to IconToggleButton class, or, better, to HidebleButton interface.

Elsewhere we can not implement unified show-hide-button logic for MapModes and ToggleDialogs in MapFrame.

Only problem is informing ToggleDialog about its button show-hide. Some events should be added to IconToggleButton (buttonShown, buttonHidden, listened in ToggleDialog) or something else...

But this need rather big refactoring (removing 4 public methods) and can lead to plugin recompilation.

Maybe I am not right, so I'll better wait for comments before starting this patch. Maybe you can make it yourself (for more stability:) )

[akks]

[remove - double post]

[akks]

I have prepared bigger patch described in previous post.

It completely removes showing-hiding button logic from ToggleDialog.

Showing-Hiding is now concentrated in HideableButton interface implemented by IconToggleButton.
Visibility management and context menus are unified now for both left toolbars (addHideContextMenu method and ListAllButtonsAction class are universal).

I did not change the signatures of ToggleDialogh class (intensively used by pluguins), but removed many methods. It does not affect the plugins, as I see. None of installed plugin crashes and full plugin recompilation works fine too.

Many methods are moved to IconToggleButton, but its existing methods are unchanged.

Please, can your review (or rework this patch) when you have enough time. (I will have very busy week myself...)

[akks]

I have prepared bigger patch described in previous post.

It completely removes showing-hiding button logic from ToggleDialog.

Showing-Hiding is now concentrated in HideableButton interface implemented by IconToggleButton.
Visibility management and context menus are unified now for both left toolbars (addHideContextMenu method and ListAllButtonsAction class are universal).

I did not change the signatures of ToggleDialogh class (intensively used by pluguins), but removed many methods. It does not affect the plugins, as I see. None of installed plugin crashes and full plugin recompilation works fine too.

Many methods are moved to IconToggleButton, but its existing methods are unchanged.

Please, can your review or rework this patch when you have enough time. I will have very busy week myself, so my answers may be slow.

[akks]

Can you please answer something to confirm that you noticed the patches :)

I am not asking for immediate patch review, it can wait for its time.

[stoecker]

Ping.

.oO(I've seen, but currently little free time, scheduled for weekend)

[akks]

OK)

[akks]

Any chance this weekend?

[stoecker]

In [4609/josm]:

fix #6953 - patch by akks - improve hide handling for left toolbar

[stoecker]

Sorry for the delay and please keep nagging if this happens in future :-)

[akks]

You did a huge amount of changes this weekend, really :) Thank you!

[stoecker]

The configuration entries depend on the translated texts. This is an unwanted behaviour and must be fixed.

[akks]

Actually, there is no untranslated text associated with button actions. Plugins can also add buttons (like PicLayer) and no one can guarantee the icons or class names will be unique. Actions are created like

super(tr("Select"), "move/move", tr("Select, move, scale and rotate objects"), .... )

where "move/move" is and icon name. If it should it be used for preferences, please edit applyButtonHiddenPreferences() and setButtonHidden(boolean b) by replacing

String actionName = (String) getSafeActionValue(AbstractAction.NAME)

with something else. I have no good idea.

[stoecker]

For menus I fixed that somehow. I think I changed the methods, so that untranslated text is passed.

[akks]

Unfortunately, IconToggleButton takes javax.swing.Action in its constructor. Sometimes it is JosmAction, sometimes - AbstractAction, sometimes - MapMode, etc. We can not change it without modifying ToggleDialog and plugins.

What can be unique text ID of any action?

[akks]

Maybe icon name would be the best solution (like it was before patching) -but all the icons must be unique and not null.

[stoecker]

Replying to akks:

but all the icons must be unique and not null.

This is anyway a requirement I would say.

[akks]

It seems I have found correct key by debugging - it is getAction().getValue("toolbar"). The patch will appear soon...

[akks]

Here it is.

By the way, some plugins implements custom showing-hiding logic (PicLayer, Routing, ...), they are generally compatible with visibilty menus and so on, but sometimes too many buttons are shown. I do not think it is a serious issue (as it does not show normally, only while reconfiguring process).

[stoecker]

Replying to akks:

By the way, some plugins implements custom showing-hiding logic (PicLayer, Routing, ...), they are generally compatible with visibilty menus and so on, but sometimes too many buttons are shown. I do not think it is a serious issue (as it does not show normally, only while reconfiguring process).

Is this really necessary? Can't we change the JOSM interface in a way, that these plugins can access the required features directly from JOSM?

We don't need to keep old workarounds alive forever.

[akks]

For now, core does not provide the functionality for dynamic showing-hiding.

Should we hide all disabled buttons (disable-enable should go to plugin logic?) or do something else?

Here is the current code from PicLayer plugin:

    /**
     * The toolbar buttons shall be active and visible only when the PicLayer is active.
     */
    @Override
    public void activeLayerChange(Layer oldLayer, Layer newLayer) {
        boolean oldPic = oldLayer instanceof PicLayerAbstract;
        boolean newPic = newLayer instanceof PicLayerAbstract;
        // actually that should be not enough - JOSM should hide all buttons that are disabled for current layer!
        if (oldPic && !newPic || oldLayer == null && !newPic) { // leave picture layer - hide all controls
            for (PicToggleButton btn : buttonList) {
                btn.writeVisible();
                btn.setVisible(false);
            }
            if (oldLayer != null)
                ((PicLayerAbstract)oldLayer).setDrawPoints(false);
        }
        if (!oldPic && newPic) { // enter picture layer - reset visibility of controls
            for (PicToggleButton btn : buttonList)
                btn.readVisible();
        }
    }

[stoecker]

I think best would be an isValidLayer() in MapMode logic or something like this.

[stoecker]

Hmm, are we talikng about MapMode or MenuToggleButtons? But for both a isValidLayer() can be useful.

[akks]

Yes, MapMode.layerIsSupported looks suitable - thank you for advice.

Please have a look at new version of patch - it hides all unsupported MapModes automatically (plugins can be easily changed to provide layerIsSupported).

This behaviour was not introduced before - try switching layers... Isn't it too radical?

[stoecker]

Yes, it is a bit radical I think. Probably we should add two additional checks:

• a hidden config enabling this behaviour for all mapmodes and
• a setHideDisabled() or something like that which activates this individual for each MapMode(), so plugins can use it.

[akks]

Here is an updated version of the patch. I noticed that no checking of layer compatibility is needed - button is disabled automatically. So the only needed functionality was to hide disabled buttons or buttons listed as 'hidden' in preferences at right moments.

Added hidden parameter is called "sidetoolbar.hideDisabledButtons" and is set to false by default.

I have also made a sample patch for Routing plugin to illustrate the work.

[stoecker]

In [4669/josm]: patch by akks - fixes for left hand buttons.

[stoecker]

See ​[o27288]. Do you want to update other plugins doing custom stuff as well?

[akks]

I can do it on weekend if ou or authors will not be faster :)

PicLayer plugin is rather complex, it is better to be fixed by author.

Only ElevationProfile and Piclayer implements custom logic (to be removed), but there are also about 5 more plugin MapModes without isLayerSupported.