Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Map Markers #3840

Merged
merged 14 commits into from
Jun 18, 2019
Merged

Custom Map Markers #3840

merged 14 commits into from
Jun 18, 2019

Conversation

deecay
Copy link
Contributor

@deecay deecay commented May 28, 2019

What type of PR is this? (check all applicable)

  • Feature

Description

Adds the feature to customize the looks of the map markers. You can change marker colors according to visualizations/ColorPalette, and marker icons with Font Awesome.

Before
58472199-b2f97280-8180-11e9-8755-3eeadf6b485c-min

After (Showing train stations with 'train' icon)
58472220-c0166180-8180-11e9-9dcf-6de1f2a4fc52-min

Related Tickets & Documents

Library used: https://github.com/marslan390/BeautifyMarker
Available icons: https://fontawesome.com/v4.7.0/icons/

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

editor (rectangle shape with 'ambulance' icon)
editor1

(dot shape, similar look to current circleMarker)
editor2-min

(circle with icon)
editor3-min

WIP Because...

  • Is Font Awesome okay? Upgrade from FontAwesome v4 to v5 may become difficult because of this feature. Icon names have changed from v4 to v5, so if we save the icon name as setting, we might need to migrate.
  • How to incorporate this with Groups? Icon per group?
  • Should we create a list of possible icon names as a list (+600)?

Copy link
Contributor

@ranbena ranbena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10x @deecay!

Some UX issues I noticed:

  1. The different markers types do not indicate the same exact location. Here's all of 'em together:

    The "X" marks the correct coordinate and only "Doughnut" seems to adhere to it.

  2. There actually is already a design related tab, which is "Map Settings" since it allows changing tile color themes. So better group 'em in one tab named "Style", with "Tile" and "Marker" headings.

  3. "Text Color" seems unclear to me cause there ain't no text. Perhaps "Foreground color" would be more suitable?

  4. I fiddled with the settings but it didn't take effect. Then I realized it's disabled by default with the checkbox on top. To avoid such confusion I suggest:
    a. Disable the color boxes when unchecked.
    b. Perhaps change checkbox label to sth like "Override default style".

  5. Let's help our users know what font icons they can use. Add a link next to "Icon Font" that points to FA4.7 icons, like we do here.

@deecay
Copy link
Contributor Author

deecay commented May 31, 2019

@ranbena, thanks for your evaluation and input.

  1. The different markers types do not indicate the same exact location.

I didn't expect this kind of behavior. Thanks for examining. This should be adjustable, so I will look into this first.

For all other suggestions, I will make additional commits after fixing #1.

@deecay
Copy link
Contributor Author

deecay commented May 31, 2019

@ranbena
Where do you think the rectangle marker should align?

The 'x' should come at the center point of the bottom edge of the rectangle? Right on, or do we want some margin between the 'x' and the box?

Another option could be that 'x' comes at the rectangle's center of gravity. Icon will hide what's at the 'x' point, but it is arguably more accurate.

WDYT?

@deecay
Copy link
Contributor Author

deecay commented May 31, 2019

In addition to the feedback, I have

  1. Added new shape type 'Circle', which was missing. Screenshot added to the original comment.

  2. Made Rectangle a square shape. And it will be positioned so that the specified coordinates comes to the center of its gravity. Screenshot updated.

  3. Added Color Name to color dropdown list.

@ranbena
Copy link
Contributor

ranbena commented Jun 15, 2019

@ranbena
Where do you think the rectangle marker should align?

The 'x' should come at the center point of the bottom edge of the rectangle? Right on, or do we want some margin between the 'x' and the box?

Another option could be that 'x' comes at the rectangle's center of gravity. Icon will hide what's at the 'x' point, but it is arguably more accurate.

WDYT?

Sorry for disappearing @deecay. Was on vacation 🏝
I think having all shapes (except for the "Marker") be in exact center makes sense.

Copy link
Contributor

@ranbena ranbena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deecay great work! Really!
I put some comments/suggestions in code.

client/app/visualizations/map/map-editor.html Show resolved Hide resolved
<div class="col-xs-6">
<div class="form-group">
<label>Icon Font
<span class="m-1-5" uib-popover-html="'Available <a href=&quot;https://fontawesome.com/v4.7.0/icons/&quot; target=&quot;_blank&quot;>icons.</a>'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to suggest a bit more verbosity. Sth like:
Screen Shot 2019-06-15 at 17 31 06

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better 😄

client/app/visualizations/map/index.js Show resolved Hide resolved
client/app/visualizations/map/index.js Show resolved Hide resolved
Copy link
Contributor

@ranbena ranbena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there!

client/app/visualizations/map/map-editor.html Outdated Show resolved Hide resolved
client/app/visualizations/map/map-editor.html Show resolved Hide resolved
client/app/visualizations/map/map-editor.html Outdated Show resolved Hide resolved
deecay and others added 2 commits June 18, 2019 00:10
@deecay
Copy link
Contributor Author

deecay commented Jun 18, 2019

@ranbena, ready for review again.

image

Copy link
Contributor

@ranbena ranbena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕺Go ahead if and remove the '[WIP]' and I'll merge.

@deecay deecay changed the title [WIP] Custom Map Markers Custom Map Markers Jun 18, 2019
@deecay
Copy link
Contributor Author

deecay commented Jun 18, 2019

Thank you @ranbena for your support.

@ranbena ranbena merged commit 99bf6d1 into getredash:master Jun 18, 2019
@ranbena
Copy link
Contributor

ranbena commented Jun 18, 2019

Thanks @deecay for putting in the time to make this a feature available to all Redash users 🍾

@deecay deecay deleted the awesome-markers branch June 19, 2019 13:52
harveyrendell pushed a commit to pushpay/redash that referenced this pull request Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants