I have some problems with the highlight JS code.

1. sometimes marker highlights don't disappear
2. highlight code is split across marker and highlight JS
3. highlight functions and var are global scope.

I have a patch

This is obviously a low priority, but it may fix some of the other issues reported RE highlights sticking.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jaxxed’s picture

Here is my refactor patch.

It takes the same base approach of using an overlay to keep a sized object, but switches to a circle (instead of rpolygon) and moves a lot of the event code into this highlight JS file.

- marker events/binds for 'markerHighlight' and 'markerUnHighlight' are used.
- highlight and unhighlight functions are now at Drupal.gmaps. and are avaiable outside of markers.

Tested and currently in use.

(This is another patch that could work for D7 modules as well)

RISK: if the highlightMarker and unhighlightMarker global functions are used in other locations they will fail (global function are out of style.) consider moving to an obj.change instead.

jaxxed’s picture

Status: Active » Needs review
podarok’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev
Priority: Minor » Normal
Status: Needs review » Needs work

all feature requests should be rolled against latest dev and after commit backported to previous version

jaxxed’s picture

Title: highligh JS refactor » highlight JS refactor
Version: 7.x-2.x-dev » 6.x-2.x-dev
Priority: Normal » Minor
Status: Needs work » Needs review

I can type

jaxxed’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev
FileSize
12.26 KB

new patch, properly done with the 7.x-2.x git source.

podarok’s picture

Status: Needs review » Fixed

#5 commited pushed to 7.x-2.x
Thanks!!!!

podarok’s picture

tagged new beta release http://drupal.org/node/1987146

jaxxed’s picture

just a heads up. I did some stress testing and found some cases where the highlight doesn't deactivate. I'll work on it more.

podarok’s picture

looking forward for possible follow-ups about to #8

jaxxed’s picture

RE: #9 : I have a change that I am looking at that solve this, but I am trying to figure out how to allow highlights for mouseovers to be separate use cases from mouseover highlights. For now I will just try to fix the problem, and worry about the additional use-cases in the future.

jaxxed’s picture

Status: Fixed » Needs work

just an update. I am still working on this, but had to change plans when I discovered that overlay draws happen again on zoom events.

podarok’s picture

Category: feature » task
Priority: Minor » Normal

looks like task
possible better title needed
or if this some kind of different task - good to create follow-up issue for minimising comment counter and better summary update

jaxxed’s picture

Status: Needs work » Needs review
FileSize
12.75 KB

Here is a completed refactor:

-fixes orphaned highlights,
-fixes zoom problems (highlights re-appear or are wrong size)
-uses a single overlay for all highlights, instead of one overlay for each highlight (no need with the v3 library as the overlay is only really used to translate pixels into meters.)

Stress tested here: http://drupal.12thirty4.com/gmaps/gmaptest/shapes

Status: Needs review » Needs work

The last submitted patch, gmap-highlightrefactor-1982292-13.patch, failed testing.

johnv’s picture

Status: Needs work » Needs review
FileSize
14.5 KB

I reported #2030397: No markers when 'Aggregate JavaScript files.' in IE8 ( due to error 'ID is expected' in highlight.js ? ) , and tried this patch to see if it resolved the problem. It doesn't :-( (the js-error and the vanishing of the markers still happen)
Otherwise, the patch does not break my site (but I only show markers, not sure what the function of highlight is.)
I couldn't apply the patch to latest dev version, mainly/only due to indentation differences. So here's a new version.

podarok’s picture

Priority: Normal » Major
Issue tags: +Needs manual testing

someone needs review this by hands with proper screenshots uploaded

podarok’s picture

Status: Needs review » Fixed

#15 commited pushed to 7.x-2.x
Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.