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

Round zoom level value in block controls #29334

Merged

Conversation

TimBroddin
Copy link
Contributor

@TimBroddin TimBroddin commented Mar 7, 2023

Fixes Automattic/wp-calypso#74158

Proposed changes:

This rounds the zoom level value shown in the Map block's controls. When zooming in/out of a Mapkit map, the camera distance gets converted to a Mapbox zoom level. This often contains lots of decimals.

This PR changes the rendering of that value in the block controls. This doesn't change the zoom value, it just changes the presentation.

CleanShot 2023-03-07 at 13 56 43@2x

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

  1. Apply this PR
  2. Set the cookie to force the display of Mapkit maps, by pasting this in your console: document.cookie = "map_provider=mapkit; path=/;";
  3. Add a post/page
  4. Add a map block
  5. Open the block properties to the right
  6. Use the zoom buttons on the map
  7. Changing the zoom should be reflected in the block's controls (might take tapping the button 2 times - because 1 zoom in Mapkit doesn't correspond to 1 zoom level in Mapbox).

It might feel counter intuitive that the value doesn't change every time you click the button, but this is a compromise.

@TimBroddin TimBroddin requested a review from a team March 7, 2023 13:02
@TimBroddin TimBroddin self-assigned this Mar 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2023

Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run

bin/jetpack-downloader test jetpack add/map-block-mapkit-zoom-value-rounding

to get started. More details: p9dueE-5Nn-p2

@github-actions github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Mar 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2023

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • 🔴 Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.


🔴 Action required: Please add missing changelog entries for the following projects: projects/plugins/jetpack

Use the Jetpack CLI tool to generate changelog entries by running the following command: jetpack changelog add.
Guidelines: /docs/writing-a-good-changelog-entry.md


Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.


Jetpack plugin:

  • Next scheduled release: April 4, 2023.
  • Scheduled code freeze: March 28, 2023.

@github-actions github-actions bot added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Mar 7, 2023
@TimBroddin TimBroddin mentioned this pull request Mar 7, 2023
3 tasks
@TimBroddin TimBroddin added [Status] Needs Team Review and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Mar 8, 2023
@github-actions github-actions bot added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Mar 9, 2023
@TimBroddin TimBroddin removed the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Mar 9, 2023
@mashikag
Copy link
Contributor

mashikag commented Mar 9, 2023

Looks good. I just noticed the strange zoom behavior when using the controls' zoom slider. Is this expected?
Screen Capture on 2023-03-09 at 17-04-20

@TimBroddin
Copy link
Contributor Author

Looks good. I just noticed the strange zoom behavior when using the controls' zoom slider. Is this expected? Screen Capture on 2023-03-09 at 17-04-20 Screen Capture on 2023-03-09 at 17-04-20

Nice catch, I'll make a ticket out of this!

@TimBroddin TimBroddin merged commit 096a0cb into add/map-block-mapkit Mar 9, 2023
@TimBroddin TimBroddin deleted the add/map-block-mapkit-zoom-value-rounding branch March 9, 2023 20:34
TimBroddin added a commit that referenced this pull request Apr 26, 2023
* Add util functions to convert from/between Mapbox zoom levels & Mapkit camera distance (#29092)

* Add helper functions to convert from & to zoom level

* Add helper functions to convert from & to zoom level

* Simplified convertCameraDistanceToZoomLevel

* Add `getMapProvider` helper (#29116)

* [not verified] Add util functions to convert from/between Mapbox zoom levels & Mapkit camera distance (#29092)

* Add helper functions to convert from & to zoom level

* Add helper functions to convert from & to zoom level

* Simplified convertCameraDistanceToZoomLevel

* [not verified] Add `getMapProvider` helper (#29116)

* Introduce useMapkitSetup hook (#29144)

* Introduce useMapkitSetup hook

* Cherry-pick

* Cherry-pick

* Add changelog

* Maps block: render Apple Maps maps on WP.com frontend (#29089)

* [not verified] WIP version of Mapkit support

* [not verified] Changelog

* Implement convertZoomLevelToCameraDistance

* - Size map according to what's inside data-map-height.
- Move resizeMapContainer to utils

* - Resize logic for Mapkit

* Rename sources to view

* Fix inclusion of React, Lodash & others

* Add support for Mapkit geocoding (#29120)

* Add Mapkit geocoding

* Add changelog

* Fix focus

* Skip POI's

* - Use async/await where possible to reduce code and increase readability

* Don't fetch user location for geosearch

* fetchKey function returns a promise

* Fallback to [] when results is null

* Don't show Mapbox token input when map provider is Mapkit (#29306)

* Hide access token panel when provider is not mapbox

* Add changelog

* Add Mapkit support in the editor (#29252)

* Cherry-pick hook

* Cherry-pick

* WIP

* Markers & info window

* WIP useEffect dependency cleanup

* - MapRef for resizing
- Better zooming for multiple markers

* Some cleanup

* - Disable zoom control on map when we have more than 1 point

* Sane defaults when adding a map

* Remove TODO

* Add changelog

* Merge with location search

* Make check for mapRef more compact

* Rename createCalloutElement to createCalloutElementCallback

* Update style code styling

* Remove todo and add linebreak between import & function call

* Code style fix

* InfoWindow renaming

* Remove commented out console.log

* Mapkit doesn't have id's for places returned, so create one

* Fix resizing doing bad things

* Revert "Add Mapkit support in the editor (#29252)" (#29353)

This reverts commit 8b79eab.

* Revert "Revert "Add Mapkit support in the editor (#29252)" (#29353)" (#29354)

This reverts commit 5ac39b1.

* Only show specific controls when mapprovider is mapbox (#29333)

* Only show specific controls when mapprovider is mapbox

* Add tests

* Add changelog

* Only load Mapkit library block once per page (#29301)

* Cherry-pick hook

* Cherry-pick

* WIP

* Markers & info window

* WIP useEffect dependency cleanup

* - MapRef for resizing
- Better zooming for multiple markers

* Some cleanup

* - Disable zoom control on map when we have more than 1 point

* Sane defaults when adding a map

* Remove TODO

* Add changelog

* Merge with location search

* Make check for mapRef more compact

* Rename createCalloutElement to createCalloutElementCallback

* Update style code styling

* Remove todo and add linebreak between import & function call

* Code style fix

* InfoWindow renaming

* Remove commented out console.log

* Mapkit doesn't have id's for places returned, so create one

* Fix resizing doing bad things

* Load Mapkit only once

* Load Mapkit only once (frontend)

* Make cookie override frontend too

* Changelog

* Reject when mapkitIsInitializing is undefined

---------

Co-authored-by: Maciej Grabowski <[email protected]>

* Recenter map after closing a marker info window (#29338)

* Cherry-pick hook

* Cherry-pick

* WIP

* Markers & info window

* WIP useEffect dependency cleanup

* - MapRef for resizing
- Better zooming for multiple markers

* Some cleanup

* - Disable zoom control on map when we have more than 1 point

* Sane defaults when adding a map

* Remove TODO

* Add changelog

* Merge with location search

* Make check for mapRef more compact

* Rename createCalloutElement to createCalloutElementCallback

* Update style code styling

* Remove todo and add linebreak between import & function call

* Code style fix

* InfoWindow renaming

* Remove commented out console.log

* Mapkit doesn't have id's for places returned, so create one

* Fix resizing doing bad things

* Recenter map when closing popup

* Changelog

* Move logic to getMapProvider (#29382)

* Move logic to getMapProvider

* Changelog

* Also make a get_map_provider function on the server side

* Change str_contains to str_pos since str_contains is only supported in PHP7.4+

* Geocode the address attribute if it's present (#29394)

* Geocode address if it's an attribute

* Changelog

* Round zoom level value in block controls (#29334)

* Cherry-pick hook

* Cherry-pick

* WIP

* Markers & info window

* WIP useEffect dependency cleanup

* - MapRef for resizing
- Better zooming for multiple markers

* Some cleanup

* - Disable zoom control on map when we have more than 1 point

* Sane defaults when adding a map

* Remove TODO

* Add changelog

* Merge with location search

* Make check for mapRef more compact

* Rename createCalloutElement to createCalloutElementCallback

* Update style code styling

* Remove todo and add linebreak between import & function call

* Code style fix

* InfoWindow renaming

* Remove commented out console.log

* Mapkit doesn't have id's for places returned, so create one

* Fix resizing doing bad things

* Round zoom value

* Catch Mapkit API request failures & show error (#29356)

* Cherry-pick hook

* Cherry-pick

* WIP

* Markers & info window

* WIP useEffect dependency cleanup

* - MapRef for resizing
- Better zooming for multiple markers

* Some cleanup

* - Disable zoom control on map when we have more than 1 point

* Sane defaults when adding a map

* Remove TODO

* Add changelog

* Merge with location search

* Make check for mapRef more compact

* Rename createCalloutElement to createCalloutElementCallback

* Update style code styling

* Remove todo and add linebreak between import & function call

* Code style fix

* InfoWindow renaming

* Remove commented out console.log

* Mapkit doesn't have id's for places returned, so create one

* Fix resizing doing bad things

* Load Mapkit only once

* Load Mapkit only once (frontend)

* Make cookie override frontend too

* Changelog

* Show error when Mapkit API request fails

* Changelog

* Friendlier error message

* Squashed commit of the following:

commit e426f62
Merge: 0f2a0f4 0712aed
Author: Tim Broddin <[email protected]>
Date:   Wed Mar 8 12:37:40 2023 +0100

    Merge branch 'trunk' into add/map-block-mapkit

commit 0712aed
Author: thingalon <[email protected]>
Date:   Wed Mar 8 20:00:50 2023 +1100

    [Boost] Clean up linting issues in the minify feature (#29352)

    * Fix linting issues in minify module

    * changelog

    ---------

    Co-authored-by: Mark George <[email protected]>

commit 12fbcf8
Author: Nauris Pūķis <[email protected]>
Date:   Wed Mar 8 09:45:29 2023 +0200

    Jetpack Boost: Add livereload (#29342)

    * Add livereload to the run dev script

    * Move livereaload functionality to "devlive" script

    * Add a bit of documentation 🤏

    * Add a changelog

commit 0dcad49
Author: Francesco Bigiarini <[email protected]>
Date:   Wed Mar 8 08:32:37 2023 +0100

    Add import package media endpoints (#29080)

    * [not verified] Add /jetpack/v4/import/media/* endpoints

    * [not verified] changelog

    * [not verified] Fix: remove categories and tags from pages.

    * [not verified] Add attachment PUT method

    * [not verified] Fix: wrong variable name

    * [not verified] Add missing body action

    * [not verified] Update version

commit cf08052
Author: thingalon <[email protected]>
Date:   Wed Mar 8 17:54:09 2023 +1100

    Add a changelog for fixing Safari lazy loading for the next boost release (#29266)

    Co-authored-by: Mark George <[email protected]>

commit 318799a
Author: Nate Weller <[email protected]>
Date:   Tue Mar 7 22:44:08 2023 -0700

    Protect: Display "All threats" instead of "All 1 threats" (#29327)

commit 39f711e
Author: Samiff <[email protected]>
Date:   Tue Mar 7 15:10:52 2023 -0700

    Update/jetpack backport 11.9 (#29345)

    * [not verified] Jetpack: 11.9 changelog and readme

    * Update stable tag to 11.9

commit e480211
Author: Matthew Reishus <[email protected]>
Date:   Tue Mar 7 14:35:16 2023 -0600

    media summary: write to memo when no images found (#29326)

commit 177950e
Author: Jason Johnston <[email protected]>
Date:   Tue Mar 7 15:25:17 2023 -0500

    VideoPress: Add/use preview hook (#29164)

    * [not verified] Add usePreview hook

    * [not verified] Use UsePreview hook

    * [not verified] Add changelog

    * Update projects/packages/videopress/src/client/block-editor/hooks/use-preview.ts

    Co-authored-by: Damián Suárez <[email protected]>

    * Move hook to a directory

    Add readme

    * Update VideoPress version

    * Fix package path

    ---------

    Co-authored-by: jhnstn <[email protected]>
    Co-authored-by: Damián Suárez <[email protected]>

commit dff2d71
Author: Nate Weller <[email protected]>
Date:   Tue Mar 7 11:33:25 2023 -0700

    Protect: More Descriptive Error Messages (#29185)

commit fc9b6b5
Author: Nate Weller <[email protected]>
Date:   Tue Mar 7 11:14:12 2023 -0700

    WAF: Minor Error Handling Improvements (#29108)

commit 128facd
Author: Damián Suárez <[email protected]>
Date:   Tue Mar 7 15:00:53 2023 -0300

    VideoPress: pick and convert core/video VideoPress instances also from inner blocks (#29339)

    * pick v6 also from inner blocks

    * changelog

    * minor code changes

    * fix changelog

    * minor doc enhancement

    * ensure to include the selected v5 instance

commit 34dc9c3
Author: Grant Kinney <[email protected]>
Date:   Tue Mar 7 09:42:49 2023 -0600

    Writing prompt block: a few cleanup tasks (#29324)

    - Uses scss variables from @automattic/jetpack-base-styles/gutenberg-base-styles where appropriate
    - Ensures "View all responses" is an external link that opens in a new tab/window
    - Ensures "prompt" is used in css classes and the like, since a block represents a single prompt

commit 76990eb
Author: Kuba Birecki <[email protected]>
Date:   Tue Mar 7 15:56:37 2023 +0100

    Implement RWD navigation for feedback dashboard (#29315)

    * [not verified] Fix RWD styles for the table and response

    * [not verified] Add navigation for switching between inbox and response views on mobile

    * Update layout component to accept custom class names

    * changelog

* Reject when mapkitIsInitializing is undefined

* Delete changelogs

---------

Co-authored-by: Maciej Grabowski <[email protected]>

* Default to the default mapCenter defined in settings.js (#29385)

* Default center

* Changelog

* Should be MutedStandard.

* Call hook to get map provider

* Read window global in getMapProvider

* Reformat error

* Allow scroll to zoom for mapkit

* Map black & white to MutedStandard on Mapkit

* Better type check

* Fix test

* Change label for Black & White option to Muted on Mapkit
Change screenshots to Mapkit version on Mapkit

* Fix minification build problem

* Fix minification build problem

* Fix minification build problem

* Mapkit token didn't work on private sites, changed this to proxy via /wp-json

* Call renamed endpoint

* Call renamed endpoint

* Remove helper class & separate endpoint for now

* Update get-map-provider to accomodate new shape of global

* Change global variable name to Jetpack_Maps

---------

Co-authored-by: Maciej Grabowski <[email protected]>
Co-authored-by: Yan Sern <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Map [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants