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

Fix map view feature #1258

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

john-michaelburke
Copy link
Collaborator

@john-michaelburke john-michaelburke commented Jul 31, 2024

Existing Bug

  • Currently if the user attempts to use --enable-map on the master branch it crashes the application.
    image

Implements

  • Re-enables the map feature such that the tab is always visible.
  • Removes the "enableMap" feature, as it is not working in the existing implementation.
  • To allow developers the ability to see the map without the token being encoded, I add a retry mechanism where it will attempt to load whatever the user had locally stored in MAPBOX_TOKEN environment variable.
  • Mutes a few verbose logs relevant to the webengine loading.
  • mapbox allows up to 50k map loads for free per month so it seems relatively harmless to always include the feature.

Options

  • I don't think the conditionally loading with the Globals.enableMap feature is the best solution, and would require a bit of QML finesse to get it working properly (mainly hiding the tab when not enabled). So one option which this PR enables is to keep it on all the time.
  • Second option is removing it entirely.
  • Third option would be to try and rework the QML to get conditional loading working which may have a bit of a learning curve. Lots of trial and error.

Testing

  • I built a test tag v4.1.21-jm-test and here is the map feature in action.
Kazam_screencast_00003.mp4

@swiftnav-travis
Copy link

Frontend and Release Workflow Started here

@swiftnav-travis
Copy link

Frontend and Release Workflow Started here

@john-michaelburke john-michaelburke marked this pull request as ready for review July 31, 2024 20:16
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.

2 participants