Ticket #1583 (closed feature: fixed)

Opened 5 years ago

Last modified 5 years ago

Bookmark Control

Reported by: hpbrantley Owned by:
Priority: minor Milestone: Addins
Component: Control Version: 2.6
Keywords: bookmark control Cc:
State:

Description

A control to store an array of bounds, as strings, referenced by a name. Clicking the name zooms to the associated bounds in the map window. The layout is control by css only. The control can be inside the map pane, outside the map pane or have no DIV at all.

The control can be told not to allow Rico to round the corners of the div.

Example:  http://www.brantleys.com/maps/bookmark.html

Attachments

bookmark.html Download (3.8 KB) - added by hpbrantley 5 years ago.
bookmark-custom.html Download (5.7 KB) - added by hpbrantley 5 years ago.
bookmark.css Download (1.6 KB) - added by hpbrantley 5 years ago.
Bookmark.js Download (15.6 KB) - added by hpbrantley 5 years ago.

Change History

Changed 5 years ago by hpbrantley

  • milestone set to 2.7 Release

Removed calls to Rico out of the control.

You now apply Rico to the div in the following manner.

bookmarks = new OpenLayers.Control.Bookmark({ 'div':OpenLayers.Util.getElement('myBookmarks'), 'title': 'Bookmarks outside the map' });
map.addControl(bookmarks);
bookmarks.add("WASHINGTON DC","-77.338126,38.639689,-76.788809,39.189005");

OpenLayers.Rico.Corner.round( bookmarks.div , {corners: "tl bl tr br", bgColor: "transparent", color: "darkblue", blend: false});
OpenLayers.Rico.Corner.changeOpacity(bookmarks.contentDiv, 0.75);

Changed 5 years ago by euzuro

  • milestone changed from 2.7 Release to Addins

this looks like a good patch, but I think it is more material for Addins directory

See  http://trac.openlayers.org/wiki/Addins

Changed 5 years ago by euzuro

Some comments:

  • APIProperties: At first glance, it seems like you might consider making the 'limit', 'stack', 'activateOnDraw', and 'clearOnDeactivate' properties APIProperties as perhaps users will want to have access to them.
  • APIMethods: same as above, but for 'maximizeControl', 'minimizeControl'
  • destroy(): generally we put this method directly below the initialize() function in the code... unless there is a clone() in which case the order is initialize(), clone(), destroy()

  • string for bbox: is there a reason why you have chosen to use strings as the bounds, both in the interface and in the storage? I think most OL users are used to using the OpenLayers.Bounds object. The way you have built the examples here (input text box) I can see maybe why you would have chosen this. However, I would imagine most people who would use this control would have some sort of point and click interface to it... in which case converting bbox's to strings only to be converted back again seems like double-work. At the very least, you should provide the user with an option to specify the bounds as an OpenLayers.Bounds object.
  • addView(): this function seems superfluous to me, especially given the above note.

Changed 5 years ago by euzuro

One more thing that occurred to me is that it might be helpful (though not necessary) to have some tests for the control.

...I think we'll have to think a bit on what's the best way to get that infrastructure set up. test.Anotherway in each addin directory? maybe.

Changed 5 years ago by elemoine

In addition to Erik's comments:

  • activate() should call the parent's method
  • activate() should return true or false (see other controls)
  • ND comments above showControls() mention LayerSwitcher

Changed 5 years ago by hpbrantley

I have applied the suggestions from Erik, except for the tests. I'll have to figure out how to write one. This is my first Javascript class.

I used some existing controls as a base for some code fragments may still remain.

I removed the activate() method. Couldn't see where it was needed.

Also, should I replace the code here or let it stay in my sandbox?

Changed 5 years ago by euzuro

yes, if you could please update this ticket with the new, final patch... that would be helpful for those of us reviewing.

Changed 5 years ago by hpbrantley

Changed 5 years ago by hpbrantley

Changed 5 years ago by hpbrantley

Changed 5 years ago by hpbrantley

Changed 5 years ago by hpbrantley

Updated control code in this ticket.

Re: Bounds as string: The original intent changed but the string remained. I have recoded the control to allow a string or bounds object to be passed.

Changed 5 years ago by euzuro

Changed 5 years ago by euzuro

  • status changed from new to closed
  • state Review deleted
  • resolution set to fixed

graduated. see source:/addins/bookmark

Note: See TracTickets for help on using tickets.