8000 fix: stabilize leaflet map by barredterra · Pull Request #16513 · frappe/frappe · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: stabilize leaflet map #16513

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

Merged
merged 1 commit into from
Apr 7, 2022
Merged

Conversation

barredterra
Copy link
Collaborator
@barredterra barredterra commented Apr 5, 2022

Before

  1. Start at the default location

  2. "Fly" to the stored location, if any

    Very annoying to see hundreds of "flights" while clicking through multiple records.

    Bildschirmaufnahme.2022-04-06.um.13.05.25.mov
  3. Else: try to geolocate the user (See video above)

    This causes annoying browser pop-ups on every form load. Also, who says that I want to map something at the user's location?

  4. Shows largely gray tiles

    I guess because the size of the control has changed since initialization. To reproduce, add some fields and a collapsible section break.

    gray-old.mov

After

  1. Start at the stored location, or, if None, at the default location

    Bildschirmaufnahme.2022-04-06.um.13.13.31.mov
  2. Don't proactively try to locate users. Who wants to be located can click the "locate" button. (See video above)

  3. Call map.invalidateSize() to make leaflet check the current size of the control and adjust accordingly - no more gray tiles!

    Bildschirmaufnahme.2022-04-06.um.13.30.34.mov

- disable flyToBounds
- invalidateSize after loading to avoid gray tiles
@barredterra barredterra requested review from a team and surajshetty3416 and removed request for a team April 5, 2022 18:48
@codecov
Copy link
codecov bot commented Apr 5, 2022

Codecov Report

Merging #16513 (4c901a7) into develop (5b7e7ef) will increase coverage by 0.35%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop   #16513      +/-   ##
===========================================
+ Coverage    56.65%   57.00%   +0.35%     
===========================================
  Files          756      756              
  Lines        67243    67242       -1     
  Branches      5799     5798       -1     
===========================================
+ Hits         38099    38334     +235     
+ Misses       25400    25393       -7     
+ Partials      3744     3515     -229     
Flag Coverage Δ
ui-tests 47.12% <0.00%> (+0.97%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@ankush
Copy link
Member
ankush commented Apr 6, 2022

"Fly" to the stored location, if any

I thought this was a feature 🤣

@surajshetty3416 surajshetty3416 added the add-visuals add before and after images/ gifs to describe the fix/ feature label Apr 6, 2022
@barredterra barredterra removed the add-visuals add before and after images/ gifs to describe the fix/ feature label Apr 6, 2022
@barredterra
Copy link
Collaborator Author

Added before/after videos.

@mergify mergify bot merged commit 81ece01 into frappe:develop Apr 7, 2022
@barredterra barredterra deleted the default-map-view branch April 7, 2022 10:27
@barredterra
Copy link
Collaborator Author

@Mergifyio backport version-13-hotfix

@mergify
Copy link
Contributor
mergify bot commented Apr 10, 2022

backport version-13-hotfix

✅ Backports have been created

surajshetty3416 pushed a commit that referenced this pull request Apr 11, 2022
Co-authored-by: barredterra <14891507+barredterra@users.noreply.github.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0