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.

Sign up for GitHub

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

Jump to bottom

refactor: Create LocationMarkerIcon component and use it in Storybook #2281

Merged
merged 4 commits into from
Nov 20, 2023
Merged

refactor: Create LocationMarkerIcon component and use it in Storybook #2281

merged 4 commits into from
Nov 20, 2023

Conversation

joshlarson
Copy link
Contributor

No description provided.

@joshlarson joshlarson requested a review from a team as a code owner November 14, 2023 20:13
@github-actions GitHub Actions
Copy link

Coverage of commit f486ca2

Summary coverage rate:
  lines......: 94.8% (3013 of 3177 lines)
  functions..: 74.6% (1259 of 1688 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

lemald
lemald reviewed Nov 14, 2023
View reviewed changes
assets/src/components/map/markers/locationMarker.tsx Outdated
)
}

export const LocationMarker = ({
Copy link
Member

Choose a reason for hiding this comment

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

suggestion (non-blocking): now that we also have the <UserLocationMarker/> component I've sort of wanted to come up with a more descriptive name for this to differentiate it (maybe something indicating that it's a geocoded location from a search result?), but just separating it out into its own module is already an improvement on its own.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

<LocationSearchMarker />?

Copy link
Member

Choose a reason for hiding this comment

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

@joshlarson I like <LocationSearchMarker/>, were you going to make that change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! And I think that's why I never got around to merging this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reminding me though - it had completely slipped my mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lemald Done!

@github-actions GitHub Actions
Copy link

Coverage of commit dba1e26

Summary coverage rate:
  lines......: 94.8% (3013 of 3177 lines)
  functions..: 74.6% (1259 of 1688 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

@joshlarson joshlarson force-pushed the jdl/feat/storybook/location-marker branch from 1d633eb to 023043d Compare November 15, 2023 14:04
@github-actions GitHub Actions
Copy link

Coverage of commit 1706843

Summary coverage rate:
  lines......: 95.0% (3019 of 3177 lines)
  functions..: 74.6% (1259 of 1688 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

Base automatically changed from jdl/feat/storybook/location-marker to master November 15, 2023 17:01
@github-actions GitHub Actions
Copy link

Coverage of commit aa49581

Summary coverage rate:
  lines......: 94.8% (3013 of 3177 lines)
  functions..: 74.6% (1259 of 1688 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

@github-actions GitHub Actions
Copy link

Coverage of commit 29dc8f3

Summary coverage rate:
  lines......: 94.8% (3013 of 3177 lines)
  functions..: 74.6% (1259 of 1688 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

@joshlarson joshlarson merged commit 469de07 into master Nov 20, 2023
8 checks passed
@joshlarson joshlarson deleted the jdl/refactor/location-marker branch November 20, 2023 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@firestack firestack firestack approved these changes

@lemald lemald lemald approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@joshlarson @lemald @firestack