Skip to content

samplePlantLocation Bug#1618

Merged
mariahosfeld merged 8 commits into
developfrom
Fix/ZoomIN_to_SinglePlantLocation
Jan 13, 2023
Merged

samplePlantLocation Bug#1618
mariahosfeld merged 8 commits into
developfrom
Fix/ZoomIN_to_SinglePlantLocation

Conversation

@sunilsabatp
Copy link
Copy Markdown
Contributor

issue-> visit https://planet-webapp-kb4pti3b8-planetapp.vercel.app/yucatan?ploc=QYVMCI and select one of the sample plant within that plantLocation

@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 20, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
planet-webapp ✅ Ready (Inspect) Visit Preview Jan 13, 2023 at 8:54AM (UTC)

@mariahosfeld
Copy link
Copy Markdown
Contributor

Functionality works for me.

Comment thread src/features/projects/components/maps/PlantLocations.tsx Outdated
Comment thread src/features/projects/styles/PlantLocation.module.scss Outdated
key={`${spl.id}-marker`}
className={`${styles.single} ${
spl.id === selectedPl?.id
spl.hid === selectedPl?.hid
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why was this change made?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@sunilsabatp Why do we compare hid instead of id here?

Comment thread src/features/projects/components/ProjectsMap.tsx Outdated
Comment thread src/features/projects/components/maps/Project.tsx Outdated
Comment thread src/features/projects/components/maps/Project.tsx
@norbertschuler
Copy link
Copy Markdown
Collaborator

  • There seems to be a loop of unnecessary API requests and some error looking at some TreeMapper data:
20221126_test.mp4

@mohitb35
Copy link
Copy Markdown
Member

mohitb35 commented Nov 28, 2022

  • There seems to be a loop of unnecessary API requests and some error looking at some TreeMapper data:

20221126_test.mp4

  1. This seems to happen when the active language is not English.
  2. While there are multiple unnecessary requests seen on the deployed preview link, it does not seem to enter a loop there. However, the loop is seen when I run this locally.
  3. It is seen on the develop branch as well, but not on main. I suggest tackling this in a separate PR.

Copy link
Copy Markdown
Member

@mohitb35 mohitb35 left a comment

Choose a reason for hiding this comment

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

The code looks mostly good to me, it resolves the specific issues of

  1. Crash on clicking on a sample tree - now the sample tree details persist after click.
  2. Clicking on the present plant location (outlined plant location) no longer zooms the user out of the map - it only shows the correct info in the project snippet.

I've left some minor comments. Once those are done, this should be good to merge.

Note: #1618 (comment) should be resolved separately. It does not seem to be linked to the fixes here.

Comment thread src/features/common/Layout/ProjectPropsContext.tsx
Comment thread src/features/projects/components/ProjectsMap.tsx
Comment thread src/features/projects/components/ProjectsMap.tsx Outdated
Comment thread src/features/projects/components/maps/Project.tsx
@mohitb35
Copy link
Copy Markdown
Member

@mariahosfeld This is approved. It can be released along with the next 12 update.

@mariahosfeld mariahosfeld merged commit 0e5711a into develop Jan 13, 2023
@mariahosfeld mariahosfeld deleted the Fix/ZoomIN_to_SinglePlantLocation branch January 13, 2023 09:10
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.

4 participants