Panel: strange behavior when using TYPE_TOOL and TYPE_BUTTON

[jorix]

Usually the buttons remain deactivate in the panels, and no problems.

But when buttons are activated, and then a tool is activated, the active buttons are also deactivated.

This occurs, for example using NavigationHistory (after moving the map) and press a tool, then the NavigationHistory are shown as deactivated but when presses button it act properly, ups!
It also happens to set up a drawing toolbar with a button to save that is activated by adding features, save button is deactivate when changing the drawing tool, ups!

The patch includes:

• Modification in panel.js
• Modification of the examples: panel.html (tool + NavigationHistory) and wfs-protocol-transactions.html (tool + save) to show strange behavior.
• Test (panel.html) extended to check buttons and new behavior.

SVN 10507

[ahocevar]

The new test cases you created in your patch are invalid - they pass with and without the patched Panel.js.

The issues you see with the NavigationHistory control is most likely because you added the NavigationHistory control to a Panel, which you  cannot do.

[ahocevar]

Replying to ahocevar:

The issues you see with the NavigationHistory control is most likely because you added the NavigationHistory control to a Panel, which you  cannot do.

Ok, this you did not do. But have you tried your modified panel.html example without your patched lib/Control/Panel.js? It works just fine. There is no issue.

Your issue with the wfs-protocol-transactions.js is that the save control (which is just a button that does not know about active or inactive) performs its action in trigger, no matter what. Currently it is the responsibility of the application developer to prevent triggering of inactive controls. Control.NavigationHistory.next/previous do this by checking the state of the stack.

So if you want to contribute a useful improvement, make Control.Panel only trigger controls that are active.

[jorix]

Replying to ahocevar:

The new test cases you created in your patch are invalid - they pass with and without the patched Panel.js.

Yes, a test on the buttons did not exist, is new. Only the last test fails with the current panel.js

The issues you see with the NavigationHistory control is most likely because you added the NavigationHistory control to a Panel, which you cannot do.

Yes, of course, but my suggestion is that you can do without problems. With the patch no problems working together NavigationHistory and TYPE_TOOL or TYPE_TOGGLE.

[ahocevar]

Sorry, I don't get it. All the tests added by your patch pass, even if the changes to Panel.js are reverted. I have just double-checked. And my comment stays the same: "if you want to contribute a useful improvement, make Control.Panel only trigger controls that are active.". I bet that this will solve your issues.

[jorix]

Replying to ahocevar:

Sorry, I don't get it. All the tests added by your patch pass, even if the changes to Panel.js are reverted. I have just double-checked...

It is true! what? I am perplexed. I looked at the evidence and found the problem, missing:

panel.addControls([buttonControl]);

At the last moment before sending first patch I changed the way to add the button. I cut but not paste.

It is a distraction, not a bad analysis of the problem. Solved the mystery I reopened the ticket.

In comment:3 ...Your issue with the wfs-protocol-transactions.js is that the save control (which is just a button that does not know about active or inactive)...

But the buttons look different when they are active. The visual effect is what worries me. This is it! Panel deactives the buttons to activate a tool, and this is not necessary.

Have you seen work my changes in example wfs-protocol-transactions.html? Does not help see (Highlighting the save button) that there are things to be saved? only this!

[ahocevar]

Repeating myself: "if you want to contribute a useful improvement, make Control.Panel only trigger controls that are active."

Visual effects are fine without your patch to Panel.js.

[jorix]

Replying to ahocevar:

Visual effects are fine without your patch to Panel.js.

I do not agree with you, there is no reason why a highlighted button turns off when switching tool.

I leave it closed, his opinion carries more weight than mine.

[ahocevar]

@jorix: I see now what you mean, but it was hard to tell from your description. I agree with your patch.

[ahocevar]

(In [10576]) Do not change the active state of button controls on a panel when a tool control is activated. p=jorix, r=me (closes #2764)

[ahocevar]

(In [10577]) Use a loop variable to avoid array lookups and use one if clause instead of two. Non-functional change (see #2764)

[jorix]

Replying to ahocevar:

(In [10576]) Do not change the active state of button controls ...

It would be nice to change NavigationHistory documentation.  outdated documentation. Do you think?

[ahocevar]

What does this ticket have to do with the NavigationHistory docs? And what, in your opinion, is outdated in that documentation? But don't answer here.

[jorix]

Replying to jorix:

It would be nice to change NavigationHistory documentation...

Sorry, I have not said anything. The documentation is compatible with the change.

[jorix]

In activateControl documentation says:

     * The action that triggers depends on the type of control, see the 
     *     description of the types of controls in the method <addControls>.

but now should refer to the constructor.

[ahocevar]

@jorix: I don't think a comment on a closed ticket will be seen by lots of eyes. Anyway, I removed the offending sentence in r10594.