I love this module. Thanks for writing and maintaining it!

I noticed a small but slightly annoying UI bug regarding the 'Show Raw Locations' setting. This setting is off by default. Instead of having the raw locations visible on page load and then using JS to hide them when so configured, it'd be a lot better to use the "js-hide" class in the raw page data, and use JS to reveal the list once JS loads.

In the non-JS case, the list will always be visible (like now).

Trivial patch against 8.x-1.x coming soon, stay tuned.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww created an issue. See original summary.

dww’s picture

Title: Google Map raw locations 'flicker' on page load instead of hiding by default » Google Map raw locations flicker on page load instead of hiding by default
Status: Active » Needs review
FileSize
2.35 KB

Status: Needs review » Needs work

The last submitted patch, 2: 2921116-1.hide-raw-locations-by-default.patch, failed testing. View results

dww’s picture

Status: Needs work » Needs review

I don't understand how those test failures relate to my patch. I didn't touch anything regarding Google Map API Keys.

Was this some weird transient test failure? Setting back to needs review to try to re-trigger the test bot...

dww’s picture

Great, both re-tests worked, so I'm leaving this at 'Needs review'. Hope you agree this is a small, easy improvement.

Thanks again!
-Derek

  • dww authored 6da06c6 on 8.x-1.x
    Issue #2921116 by dww: Google Map raw locations flicker on page load...
ChristianAdamski’s picture

Status: Needs review » Fixed

Committed slightly altered.

dww’s picture

Thanks!

dww’s picture

Hrm, looking more closely at the patch you committed, now the raw locations will always be invisible for the non-JS case. Was that intentional? My patch tried to preserve the behavior for folks with JS disabled or otherwise unavailable. After commit 6da06c68587 those people will never see the raw locations, no matter what, since the original markup sent down the wire uses display: none and we only show it via JS. My approach sent the locations via a method that is only hidden if JS is enabled.

Should I re-open this with another patch, or should I forget it and leave the non-JS people in the dark? ;)

Thanks!
-Derek

Status: Fixed » Closed (fixed)

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