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

PIMS-288 Parcel Details Change Unexpectedly Pt.2 #1926

Merged
merged 17 commits into from
Dec 4, 2023

Conversation

dbarkowsky
Copy link
Collaborator

@dbarkowsky dbarkowsky commented Nov 23, 2023

🎯 Summary

PIMS-288

This is a follow up PR to the original PIMS-288 #1917

Feedback from PO was that parcel details should only update when using the dropped marker, not just when in edit mode.

Current Status

I have it back in the original state before PR #1460, where I think this issue started.
This means that only the lat and long update with the marker, but fields aren't auto-populated.

The drop pin seems to be working with the parcels. Info only changes when using the pin, not otherwise.
With buildings, there's an issue where using the drop pin picks up the info from the previous click. Not sure where it's being set or how to track it down.

Issue with buildings fixed. It was setting the namespace to undefined before the location was called. One more reason to move away from this giant formik state in the future.

Changes

The pins in the sidemenu for buildings and for parcels now update the location and some details of the properties only when picked up and dropped.

I ended up making a universal pin component. There actually used to be one, but it was taken out at some point.

Testing

There are three places these pins exist:

  • Building form in edit mode on the first Building Details tab.
  • Land form in edit mode on the Parcel Details tab.
  • Land form when adding a new Parcel. You have to choose the second option "Select Parcel from Map"

Issues

Two things I would like to fix, but haven't figured out how:

  • The zoom and centre when dropping the pin is off. It positions the view too far to the left. Not sure where this is happening though, and the issue exists in DEV already.
  • I couldn't get the second test in MapDropPin.test.tsx to run. It would be nice to have, but technically it's already getting tested in some of the tests for parent components.

TODO

  • Make fields populate on click, but only when dropping the marker pin.
  • Marker pin should work the same on both LandForm and BuildingForm
  • Fix bug where building pin jumps to previous click, not current click.
  • Consolidate drop pins into a uniform component.
  • Removed styled-components elements.
  • Update text for user.
  • Add tests for coverage

🔰 Checklist

  • I have read and agree with the following checklist and am following the guidelines in our Code of Conduct document.
  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation where required.
  • I have tested my changes to the best of my ability.
  • My changes generate no new warnings.

Copy link

🚀 Deployment Information

The APP Image has been built with the tag: 1926. Please make sure to utilize this specific tag when promoting these changes to the TEST and PROD environments during the APP deployment. For more updates please monitor Image Tags Page on Wiki.

Copy link

codeclimate bot commented Nov 23, 2023

Code Climate has analyzed commit 4e0d74a and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 76.4% (50% is the threshold).

This pull request will bring the total coverage in the repository to 55.8%.

View more on Code Climate.

@github-actions github-actions bot added the Tests label Dec 1, 2023
Copy link
Collaborator

@TaylorFries TaylorFries left a comment

Choose a reason for hiding this comment

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

dropping pin onto map updates some fields in all three spots. I think that the zoom issue is something larger than this ticket and should be investigated separately.

@dbarkowsky dbarkowsky merged commit 01fd84b into main Dec 4, 2023
7 checks passed
@dbarkowsky dbarkowsky deleted the PIMS-288-Parcel-Details-Change-Unexpectedly branch December 4, 2023 18:01
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.

2 participants