Skip to content

Add optional pin marker icon#165

Merged
allbecauseyoutoldmeso merged 2 commits intomainfrom
kg-marker-pin
Jul 28, 2022
Merged

Add optional pin marker icon#165
allbecauseyoutoldmeso merged 2 commits intomainfrom
kg-marker-pin

Conversation

@allbecauseyoutoldmeso
Copy link
Copy Markdown
Contributor

@allbecauseyoutoldmeso allbecauseyoutoldmeso commented Jul 26, 2022

What

  • Add optional markerImage argument that defaults to 'circle'.
  • If markerImage="pin" is set render pin instead of circle.

Storycard

https://trello.com/c/FexsTSCa/1020-red-line-show-the-a-pin-for-the-identified-property-according-to-map-information

Screenshot

Screenshot 2022-07-26 at 16 27 57

@netlify
Copy link
Copy Markdown

netlify Bot commented Jul 26, 2022

Deploy Preview for oslmap ready!

Name Link
🔨 Latest commit a327c4f
🔍 Latest deploy log https://app.netlify.com/sites/oslmap/deploys/62e250a66870e000099e321b
😎 Deploy Preview https://deploy-preview-165--oslmap.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@allbecauseyoutoldmeso allbecauseyoutoldmeso self-assigned this Jul 26, 2022
@allbecauseyoutoldmeso allbecauseyoutoldmeso force-pushed the kg-marker-pin branch 5 times, most recently from 27bfd3c to c63500d Compare July 27, 2022 09:53
- Add optional markerImage argument that defaults to 'circle'.
- If 'pin' is shown render pin instead of circle.
Copy link
Copy Markdown
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

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

Thanks, Kate!! A very welcome improvement on the basic circle 🌚

Left one comment/suggestion - happy to chat if anything is unclear!

Comment thread src/components/my-map/index.ts Outdated
useScaleBarStyle = false;

@property({ type: String })
markerImage = null;
Copy link
Copy Markdown
Member

@jessicamcinchak jessicamcinchak Jul 27, 2022

Choose a reason for hiding this comment

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

Can markerImage have non-null default value here?

Lit doesn't natively support an "Enum" property type, but we have a couple other example properties (drawPointer is a good one I think) where we've defined a typescript type to act as a pseudo-enum for string property types, which improves error handling and dev experience with smarter code editor suggestions. I think markerImage could be a good fit for that pattern too if you don't mind tweaking?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done! I like this pattern - and I think will be especially nice if we introduce other marker icons!

@allbecauseyoutoldmeso allbecauseyoutoldmeso merged commit dff78e9 into main Jul 28, 2022
@allbecauseyoutoldmeso allbecauseyoutoldmeso deleted the kg-marker-pin branch July 28, 2022 09:37
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.

2 participants