Scale control: Implement methods activate/deactivate.

[jorix]

The attached patch implements the methods activate/deactivate on the control.

The patch includes a specific test.

The tests work well in: FF368, IE8, Chrome6 & Safari5.

[jorix]

In the new patch moves the creation of "element" from draw to initialize.
Also it controls whether "element" comes from the initialize parameter (with (this.div === this.element) due to the assignment this.div = this.element; in initialize).

[ahocevar]

Same situation as with #2519 and #2831.

[jorix]

Replying to ahocevar:

Same situation as with #2519 and #2831.

Also added missing "destroy".
Same as in #2831 that missing "unregister" for 'changelayer' and 'changebaselayer' in "destroy"

[ahocevar]

And the fix to the destroy method is related to the title of this ticket in which way?

[jorix]

Replying to ahocevar:

And the fix to the destroy method is related to the title of this ticket in which way?

I do not seemed relevant, is just a justification of the increase of code.

On the other hand, in 3.0 all code into "if" of the type "if (this.div === this.element) {" can be removed.
I put a comment in the code to find them easily?

I can reopen this ticket for the same reason that #2831 was reopened?

[ahocevar]

Interesting. jorix seems to know more about 3.0 than we do. Feel free to reopen and find someone to review your patch.

[jorix]

Replying to ahocevar:

Interesting. jorix seems to know more about 3.0 than we do. Feel free to reopen and find someone to review your patch.

No, no, please!
I only followed very carefully everything that refers to 3.0: "3.0 IRC Meeting", performance, shorter code, GitHub and the ideas presented by Tim talking about constructor signatures (in Popups: 6 args!, or more. The sixth in "Popup.js" is the same as the seventh in "FramedCloud.js", what a nightmare!)
We arrive at the single signature? I do not know. But it would be good to eliminate optional args. And desirable that derived classes have the same number of arguments as base class.

...

Thanks, I reopen this ticket

[ahocevar]

This this.div == this.element stuff is so ugly! Why can't you just add this.div to this.element on activate, and remove it from its this.element parent on deactivate? Also, since no events are attached to this.div, there is no need to perform any destroy action.

[jorix]

Replying to ahocevar:

This this.div == this.element stuff is so ugly!

Yes you are right, is ugly.
I thought about putting a flag like "elelemtIsArgument" but I looked ugly, and I put something uglier.
The flag is required when "element" is an argument, when control is activated the "element" is added as a child of "div" and the scale out the map disappears and appears inside the map
How would you like a flag?
Or is it better to reformulate the constructor removing the argument "element" and put the ticket in Milestone 3.0?

[jorix]

Replying to jorix:

Replying to ahocevar:

This this.div == this.element stuff is so ugly!

Yes you are right, is ugly...

I thought that "this.div = this.element" could be replaced by
"this.div = this.element.parentNode" (also ugly), but has the advantage that can remove the "if" in "activate", "deactivate" and "destroy"
Is acceptable?

[jorix]

I do not see an elegant way to address the problem of the argument "element". I close it.

Note: I think the questions in the comment:11 are valid, but comment:12 is not valid, causes more problems than it solves.