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

Media&Text: round position attribute on focal point save #33915

Merged
merged 3 commits into from
Aug 2, 2022

Conversation

walbo
Copy link
Member

@walbo walbo commented Aug 5, 2021

Description

Currently, the focal point is not rounded in the Media & text block. That makes the block markup contain styles with many decimal places.

This PR fixes the problem and adds deprecation logic to keep current blocks valid.

Same logic was added to the cover block in #19183

Fixes #33615

How has this been tested?

  1. On the trunk branch, I inserted a media text block with an image and enabled "Crop image to fill entire column".
  2. I set the focal point coordinates to left 56% and top 57%.
  3. I checked on the code editor that the background-position was 56.00000000000001% 56.99999999999999%.
  4. I saved the post.
  5. I switched to this branch and reloaded the post.
  6. I verified the block was still valid and in the code editor I verified the background-position was now 56% 57%.
  7. I inserted another block with same attributes and verified in the code editor that the position was 56% 57%.

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@Mamaduka
Copy link
Member

Hi, @walbo

Sorry, it looks like this PR fell through the cracks.

Do you mind rebasing this? I would be happy to help with testing 🙇

Thank you

@walbo
Copy link
Member Author

walbo commented Jul 28, 2022

Hi, @walbo

Sorry, it looks like this PR fell through the cracks.

Do you mind rebasing this? I would be happy to help with testing 🙇

Thank you

Yep 👍 Don't have time this week, but can fix it next week. Will tag you for a review when ready.

@walbo walbo force-pushed the fix/media-text-focal-point branch from 9fe1ae3 to 682b46a Compare August 1, 2022 19:55
Petter Walbø Johnsgård added 2 commits August 1, 2022 21:56
@walbo walbo requested a review from Mamaduka August 1, 2022 20:34
@walbo
Copy link
Member Author

walbo commented Aug 1, 2022

@Mamaduka Should be rebased and updated now. Sorry for the big changeset in the deprecated file. Made v1-v5 variables instead of having an huge array.

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Thank you, @walbo!

The deprecation clean-up looks good.

I tested new deprecation with a few different block setups from the pattern library, and there are no issues ✅

@walbo walbo merged commit 32aeeaa into WordPress:trunk Aug 2, 2022
@walbo walbo deleted the fix/media-text-focal-point branch August 2, 2022 13:19
@walbo
Copy link
Member Author

walbo commented Aug 2, 2022

Thanks for the review and the help on testing 👍

@github-actions github-actions bot added this to the Gutenberg 13.9 milestone Aug 2, 2022
@oandregal oandregal changed the title Block library: Round position attribute on media-text focal point save Media&Text: round position attribute on focal point save Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Media & Text Affects the Media & Text Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Media & Text Focal Point Picker Producing Long Decimal Values
2 participants