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

Add additional marker colors and icons #17

Open
wants to merge 49 commits into
base: master
Choose a base branch
from

Conversation

vikasrana1989
Copy link
Contributor

No description provided.

@@ -260,6 +289,8 @@ export default Ember.Component.extend(ParentMixin, {
event.stop();
});
listeners.pushObjects([ mapListener, dataListener ]);
}else if (tool.dataId === 'Point') {
Copy link
Owner

Choose a reason for hiding this comment

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

Style nitpick

let isDefaultIcon = tool.icon.id === tool.default.icon.id;
let isDefaultColor = tool.customColor.id === tool.default.customColor.id;

if (! isDefaultIcon || ! isDefaultColor) {
Copy link
Owner

Choose a reason for hiding this comment

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

if (!isDefaultIcon || !isDefaultColor) {

</div>
{{/if}}
{{#if (is-equal opt.type 'customColor')}}
<div class="dropdown {{if customColorMenu 'open'}}">
Copy link
Owner

Choose a reason for hiding this comment

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

consistent naming between iconDropDown and customColorMenu, since both are dropdowns

id: 'pin',
display: 'Pin'
}],
icon:{
Copy link
Owner

Choose a reason for hiding this comment

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

spacing

Copy link
Owner

Choose a reason for hiding this comment

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

Not sure about putting the defaults and the list items in this config.

@knownasilya
Copy link
Owner

Icons http://map-icons.com/

@vikasrana1989
Copy link
Contributor Author

vikasrana1989 commented Apr 19, 2017

@knownasilya

  1. We have to add ember drop down component to add the customize drop down. Please check the latest code.
  2. We added the drop down for color as same as it was for others.
  3. We added a new drop down for marker icons. There are some style issue in icon drop. We will fix those by Tomorrow.
  4. We tried http://map-icons.com/ but there were some problem in there. We checked and found that http://map-icons.com/ is not maintained any more. And owner suggested to use https://material.io/icons/
  5. Now we are review https://material.io/icons/ to update icon color on map.

package.json Outdated
"broccoli-asset-rev": "^2.4.5",
"broccoli-merge-trees": "^1.2.1",
"broccoli-static-compiler": "^0.2.1",
"ember-bootstrap": "1.0.0-alpha.10",
Copy link
Owner

Choose a reason for hiding this comment

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

I'd rather not use this heavy addon since we don't use it anywhere else in MapGeo, and most likely we'll be updating to material-design or BS4 in the near future.

@knownasilya
Copy link
Owner

knownasilya commented Apr 19, 2017

  1. I'd rather not use that addon, maybe just use a select instead of the dropdown.
  2. Excellent!
  3. Great!
  4. See below.
  5. https://material.io/icons/ is fine (I actually want to convert mapgeo to material icons/design eventually), just add the icons in the map section.

<select onchange={{action "updateOptionValue" activeTool 'icon' value="target.value"}}>
{{#each activeTool.icons as |icon|}}
<option value={{icon.display}}>
<div>
<img src={{icon.path}} height="10px" width="10px">
Copy link
Owner

Choose a reason for hiding this comment

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

You should be able to render the svgs directly without making a file.

{{#dd.menu as |ddm|}}
{{#each activeTool.icons as |icon|}}
<div {{action 'updateOptionValue' activeTool 'icon' icon}}>
<select onchange={{action "updateOptionValue" activeTool 'icon' value="target.value"}}>
Copy link
Owner

Choose a reason for hiding this comment

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

Looking good 👍

@knownasilya
Copy link
Owner

knownasilya commented Apr 21, 2017

Just tried out the latest changes. Things look good. A few things came to mind:

  1. The marker dropdown resets to the default after every use of the marker tool
  2. The SVG markers could probably be scaled down a bit
  3. I'm not sure about having a select for the hover color, we can talk about this in the meeting
  4. We can probably remove the default marker, since it just clashes, and make the pin default

@knownasilya
Copy link
Owner

knownasilya commented Apr 21, 2017

Complementary function: http://stackoverflow.com/a/37657940/483616
Select addon: http://www.ember-power-select.com/ (should work for marker display)

@vikasrana1989 vikasrana1989 changed the title No Merge: Add additional marker colors and icons Add additional marker colors and icons Apr 26, 2017
icon: {
id: 'default',
display: 'Default',
path: 'google-maps-markup/images/spotlight-poi-highlighted_hdpi.png',
Copy link
Owner

Choose a reason for hiding this comment

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

This icon needs to be removed.

@vikasrana1989
Copy link
Contributor Author

Issues Description:-

  1. Marker Icon position is not coming right some time and not properly centered.
  2. When we are removing markers, Marker Icons are not getting remove from the map.

Both Issues are fixed and latest code have been updated here. This branch is ready to merge. We are clear now.

@knownasilya knownasilya added this to the v3.0 milestone Oct 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants