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

Apple Maps block gives error in WP 6.4-beta2 if MapKit Credentials is not configured. #182

Closed
1 task done
iamdharmesh opened this issue Oct 4, 2023 · 4 comments · Fixed by #183
Closed
1 task done
Assignees
Labels
type:bug Something isn't working.
Milestone

Comments

@iamdharmesh
Copy link
Member

Describe the bug

While debugging on E2E test failure on the WP trunk (WP 6.4-beta), I found that the Apple Maps block gives This block has encountered an error and cannot be previewed. error in WP 6.4-beta2 if MapKit Credentials is not configured.

Based on initial findings it seems, the issue is caused due to recent accessibility improvements in placeholder instructions (WordPress/gutenberg#45801), The placeholder component expects raw string in instructions but the plugin passes the <IsAdmin> component, which causes the issue with call to speak function and block encounter an error.

Steps to Reproduce

  1. Install and activate the plugin on WP 6.4-beta-2
  2. Keep Mapkit js credentials blank
  3. Go to create post/page and add "Apple Maps" block
  4. Notice error This block has encountered an error and cannot be previewed. error

Screenshots, screen recording, code snippet

image

Environment information

No response

WordPress information

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@iamdharmesh iamdharmesh added the type:bug Something isn't working. label Oct 4, 2023
@iamdharmesh iamdharmesh mentioned this issue Oct 4, 2023
4 tasks
@fabiankaegy
Copy link
Member

Great find on what's causing the issue. Let's see if we can help fix this in core before the 6.4 release.

@iamdharmesh iamdharmesh self-assigned this Oct 4, 2023
@iamdharmesh
Copy link
Member Author

@fabiankaegy Based on comment here it needs to be fixed here in maps-block-apple as well. right? What do you think?

The instructions prop is meant to accept raw strings. Instead, in your code you're passing a React element. It may have accidentally worked in previous versions of the Gutenberg plugin/WordPress, but the prop was always meant to be a string.

@fabiankaegy
Copy link
Member

Ahh yeah, I had missed that in the comments. That is annoying. Thanks for pointing it out.

In that case yeah we will need to precompute all that logic and then only pass in the processed string.

@iamdharmesh
Copy link
Member Author

@fabiankaegy I have raised a fix PR for this with a slightly different approach by dropping the usage of the instructions prop of Placeholder. Instructions contain a link to Mapkit docs and I didn't find any way to add link instructions while keeping instructions as a raw string (Maybe there is a way but I am not aware of it). Please let me know if you think we could have a better approach here.

Thanks

@iamdharmesh iamdharmesh added this to the Future Release milestone Oct 4, 2023
@iamdharmesh iamdharmesh moved this from Incoming to In Review in Open Source Practice Oct 4, 2023
@jeffpaul jeffpaul modified the milestones: Future Release, 1.2.0 Oct 4, 2023
@github-project-automation github-project-automation bot moved this from In Review to Merged in Open Source Practice Oct 6, 2023
@dkotter dkotter modified the milestones: 1.2.0, 1.1.2 Oct 13, 2023
@vikrampm1 vikrampm1 moved this from Merged to Done/Released in Open Source Practice Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants