Skip to content

fix: add prop to set unique id on element & clear the featureSource unless clickFeatures is enabled#110

Merged
jessicamcinchak merged 5 commits intomainfrom
jess/debugging
Jan 14, 2022
Merged

fix: add prop to set unique id on element & clear the featureSource unless clickFeatures is enabled#110
jessicamcinchak merged 5 commits intomainfrom
jess/debugging

Conversation

@jessicamcinchak
Copy link
Copy Markdown
Member

@jessicamcinchak jessicamcinchak commented Jan 13, 2022 •
edited
Loading

working solution:

  • add an id property so we can set a custom id on <my-map /> element, defaults to "map" as before
  • clear the featureSource upfront when only showFeaturesAtPoint is enabled, which should prevent prior query results from persisting in the layer. if showFeaturesAtPoint && clickFeatures are enabled (don't know of any prod uses of this yet), then we don't clear the source as before to allow for selecting/deselecting features on click
  • clear the drawingSource upfront for both cases: when we want to start drawing fresh AND when we want to load a previously drawn polygon into the drawingSource to modify. previously, we only cleared it in the case of starting a fresh drawing

dugging notes:
update index.html to something like this:

<body>
  <div style="display: flex; flex-direction: column;">
    <div style="margin-bottom: 1em">
      <my-map showFeaturesAtPoint latitude="51.507351" longitude="-0.127758" id="map-1" />
    </div>
    <div>
      <my-map showFeaturesAtPoint latitude="51.506351" longitude="-0.124758" id="map-2" />
    </div>
  </div>
</body>

in short: when rendering the component multiple times on a page with different props, each instance reflects the layer data of the other - some props sync unexpectedly (ie showFeaturesAtPoint, drawMode), and others do not (featureColor, etc).

expected behavior is for each instance of the component to exclusively reflects its' own props & it's own layer data based on those props (eg only the feature that intersects with its' unique lat/lng).

this can be reproduced in PlanX by selecting an address, going 'back' to change the address, and then seeing two highlighted features on the FindProperty map

things i learned:

@netlify
Copy link
Copy Markdown

netlify Bot commented Jan 13, 2022 •
edited
Loading

✔️ Deploy Preview for oslmap ready!

🔨 Explore the source changes: bc793a2

🔍 Inspect the deploy log: https://app.netlify.com/sites/oslmap/deploys/61e14859383fc00008694ff3

😎 Browse the preview: https://deploy-preview-110--oslmap.netlify.app

@jessicamcinchak jessicamcinchak changed the title wip: debugging fix: ability to set unique id on component & clear the featureSource unless clickFeatures is enabled Jan 13, 2022
@jessicamcinchak jessicamcinchak changed the title fix: ability to set unique id on component & clear the featureSource unless clickFeatures is enabled fix: add prop to set unique id on element & clear the featureSource unless clickFeatures is enabled Jan 13, 2022
@jessicamcinchak jessicamcinchak marked this pull request as ready for review January 13, 2022 17:34
@jessicamcinchak jessicamcinchak requested a review from a team January 13, 2022 17:34
@jessicamcinchak
Copy link
Copy Markdown
Member Author

jessicamcinchak commented Jan 14, 2022

About the property map returned two unrelated polygons

@jessicamcinchak
Copy link
Copy Markdown
Member Author

jessicamcinchak commented Jan 14, 2022

Super zoomed out map on 'find property'

@jessicamcinchak
Copy link
Copy Markdown
Member Author

jessicamcinchak commented Jan 14, 2022

Map remembers a previously searched property

DafyddLlyr
DafyddLlyr approved these changes Jan 14, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

All the changes here look good and make sense to me 👍

@jessicamcinchak jessicamcinchak merged commit b675c9f into main Jan 14, 2022
@jessicamcinchak jessicamcinchak deleted the jess/debugging branch January 14, 2022 10:44
@jessicamcinchak jessicamcinchak mentioned this pull request Jan 14, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DafyddLlyr DafyddLlyr DafyddLlyr 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.

2 participants

@jessicamcinchak @DafyddLlyr