Skip to content

Feature/map view rewrite#1805

Open
altair1673 wants to merge 327 commits into
masterfrom
feature/map-view-rewrite
Open

Feature/map view rewrite#1805
altair1673 wants to merge 327 commits into
masterfrom
feature/map-view-rewrite

Conversation

@altair1673

Copy link
Copy Markdown
Contributor

Prerequisites

  • Reviewed the checklist

  • Reviewed feedback from the "Sonar Cloud" bot. Note that you have to wait
    for the "CI / Unit Tests") to complete first. Failed Unit tests can be
    debugged by adding the label "verbose logging" to the GitHub PR.

Description of the Change

A new MapView has been implemented in JavaFX. It has most of the features of the old MapView but with a modern look. This view uses a vector graphic to avoid pixilation when infinitely zooming. Some features have been reworked and work differently from the old MapView such as the heatmap layers. Some new versions of the drawing features and clustering will need to be made in the future to make them more useful. The design of the "Zoom to Location" UI needs to look better but the functionality works. This new MapView is more responsive to user input, no "cheat-code" type mouse click patterns needs to be made to get its features to work. The code is also a lot cleaner now.

Alternate Designs

Why Should This Be In Core?

Implements the new top component framework. Is more modern looking. The old MapView used OpenGL calls which was crashing with the actual graph display which also used OpenGL.

Benefits

Friendlier UI.
Responsive to inputs.
Will throw errors at user and not just crash.
Clean readable code.
Code documented.
More tests than old MapView.

Possible Drawbacks

Due to this implementation being with a vector graphic instead a lot of work around had to be made when implementing many features. Some might not work exactly 100% like previous features but will be close enough.

Verification Process

Unit tests are still being written and there is quite a lot of testing to do still. Unit testing graphical elements on screen is not straightforward and will take some time.

Applicable Issues

#1746

serpens24 and others added 26 commits December 18, 2023 10:52
@Delphinus8821

Copy link
Copy Markdown
Collaborator

Gave it a test and the functionality is working well overall, just a few small parts that aren't quite working as expected.

  • Measure on the Tools Overlay can't currently be used to measure between points under markers
  • Cluster markers are not lined up vertically on the markers on the map
  • The help button doesn't link to the Map View help page
  • The export to GeoPackage and Shapefile buttons throw errors

@antares1470 antares1470 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A few additional things from testing:

  1. On a small screen, you can't access the entire menu from the default view width (on mine I can't see the help button, nor can I get to it)
  2. Both left and right mouse click currently do map panning, left click should do box select (as with the current map view)
  3. With the Info Overlay enabled, there are a couple observations from a small screen. Firstly the width of the map dropdown blows out to the largest width when turned on (possibly that dropdown and the overlay are connected in layout somehow?). Secondly, I can't see a chunk of the map with it on unless I widen the view (likely due to being tied with the width of the menu bar)
  4. In the Color dropdown, "Ovelay" -> "Overlay"
  5. For the Export Menu, can we remove the checkboxes next to the export options? I think their inclusion is a bit confusing given what those options do

@github-actions

github-actions Bot commented Dec 1, 2024

Copy link
Copy Markdown

This pull request is stale because it has been open for 6 months with no activity. Consider reviewing and taking an action on this pull request.

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.

5 participants