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

Feature/5713 add hover tip for spatial extents #74

Merged
merged 10 commits into from
Jul 10, 2024

Conversation

NekoLyn
Copy link
Contributor

@NekoLyn NekoLyn commented Jul 5, 2024

  1. fix showing overlapping spatial extents when click on a single unclustered point
  2. create a simple popup for hovering on a single unclustered point (different from design which is too big and contains too much information, and may need change later according to Steve @quye04 )
  3. adjust search page height to be adaptive to the screen height
  4. adjust the cluster size to make less unclustered points on map

@NekoLyn NekoLyn marked this pull request as draft July 8, 2024 00:35
@NekoLyn NekoLyn marked this pull request as ready for review July 9, 2024 01:45
@vietnguyengit
Copy link
Contributor

I'm reviewing it's a long one @utas-raymondng , FYI so you won't do duplicate work.

@NekoLyn if you requested me and I'm in it, make sure to communicate before you request another person.

src/components/map/mapbox/popup/MapPopup.tsx Outdated Show resolved Hide resolved
src/components/map/mapbox/layers/ClusterLayer.tsx Outdated Show resolved Hide resolved
src/components/map/mapbox/layers/ClusterLayer.tsx Outdated Show resolved Hide resolved
src/components/map/mapbox/layers/ClusterLayer.tsx Outdated Show resolved Hide resolved
src/components/map/mapbox/layers/ClusterLayer.tsx Outdated Show resolved Hide resolved
src/components/map/mapbox/layers/ClusterLayer.tsx Outdated Show resolved Hide resolved
src/components/map/mapbox/popup/MapPopup.tsx Show resolved Hide resolved
@utas-raymondng
Copy link
Contributor

@NekoLyn remove me from reviewer, I let @vietnguyengit do it this time

@NekoLyn NekoLyn removed the request for review from utas-raymondng July 9, 2024 07:10
@NekoLyn
Copy link
Contributor Author

NekoLyn commented Jul 10, 2024

@NekoLyn remove me from reviewer, I let @vietnguyengit do it this time

somehow cannot remove you from reviewer since you have made comments...I think one approvement is enough maybe..If it needs your approvement I will ping you

@vietnguyengit
Copy link
Contributor

I saw you've ticked "Resolved" for the comments but I cannot find new commits or new changes to some comments requiring code changes @NekoLyn

@NekoLyn NekoLyn requested a review from vietnguyengit July 10, 2024 01:59
@NekoLyn
Copy link
Contributor Author

NekoLyn commented Jul 10, 2024

I saw you've ticked "Resolved" for the comments but I cannot find new commits or new changes to some comments requiring code changes @NekoLyn

Hi I just request the review. Had some problem with pc just restarted it and commit...

Copy link
Contributor

@utas-raymondng utas-raymondng left a comment

Choose a reason for hiding this comment

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

I leave this to @vietnguyengit

@utas-raymondng utas-raymondng merged commit 487b63e into main Jul 10, 2024
2 checks passed
@vietnguyengit
Copy link
Contributor

just wonder why your commits always have main merged line ? @NekoLyn

@vietnguyengit
Copy link
Contributor

was that git asking you to rebase or you choose to do so? @NekoLyn

@vietnguyengit vietnguyengit deleted the feature/5713-add-hover-tip-for-spatial-extents branch July 10, 2024 05:49
@NekoLyn
Copy link
Contributor Author

NekoLyn commented Jul 10, 2024

was that git asking you to rebase or you choose to do so? @NekoLyn

I choose to do it. but githut will also detect if the commit is ahead or behind 'main'. if the commits are ahead of 'main', then someone can merge the pr. however if the commits are behind 'main' then you need to rebase before merge, which may casue unsolved conflicts. That's why i will check any updates of 'main' and merge then resolve potential confilcts before pushing commit.

@NekoLyn
Copy link
Contributor Author

NekoLyn commented Jul 10, 2024

just wonder why your commits always have main merged line ? @NekoLyn

This line is I merge lastest pr that mereged to 'main'. The commit message is different from Raymond and Havier due to using different IDE (I am using VS code and they may use the Intellij)

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.

3 participants