Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Add "FocalPointPicker" to Featured Product block #544

Merged
merged 5 commits into from
May 3, 2019

Conversation

ryelle
Copy link
Member

@ryelle ryelle commented Apr 29, 2019

Fixes #403 - Adds the FocalPointPicker to the Featured Product block, which lets you reposition the background image. For example, by default this person's head is cut off:

before

But by using the focal point picker, we can move the "focus" up a bit and have this:

after

Accessibility

FocalPointPicker is a core component, it has the 2 text inputs as accessible fallbacks from the image selection. There's a minor bug with the inputs, will be fixed with WordPress/gutenberg#15255

How to test the changes in this Pull Request:

  1. Add a Featured Product block (most noticeable change when the image is tall)
  2. Use the Focal Point Picker to change which part of the image is visible in the block
  3. Publish the post, reload the page – it should keep that part of the image visible
  4. View it on the frontend – it should look the same on the frontend

@ryelle ryelle added this to the 2.1 milestone Apr 29, 2019
@ryelle ryelle self-assigned this Apr 29, 2019
@ryelle ryelle requested a review from a team April 29, 2019 19:43
Copy link
Contributor

@timmyc timmyc left a comment

Choose a reason for hiding this comment

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

While testing this out - I get a JS exception and block-editor error after selecting a product:

image

If I click "Attempt Recovery" the block appears, but then if I try to interact with the block, the error is shown again.

@ryelle
Copy link
Member Author

ryelle commented Apr 29, 2019

@timmyc Are you using the gutenberg plugin? I think maybe the FocalPointPicker isn't available in WP core yet (I didn't check this before doing the PR)… so maybe this needs to wait until that component is released in WP core.

@timmyc
Copy link
Contributor

timmyc commented Apr 30, 2019

Are you using the gutenberg plugin?

No I currently only have WooCommerce 3.6.2, WC Smooth Generator ( to build out 100+ products for the grid PR ;) ) and this branch installed/active - so that could likely be the cause. Should I checkout g-berg master, or is it in the current plugin?

Is FocalPointPicker set to be released to WP Core in 5.2?

@ryelle
Copy link
Member Author

ryelle commented Apr 30, 2019

Is FocalPointPicker set to be released to WP Core in 5.2?

It looks like it is, yeah - I deactivated GB on my test site with 5.2 RC and it's working. Looks like 5.2 will be out on May 7, but we probably want to keep support parity with WC core so we can easily merge, so I think that's 5.1 + 5.2 (current + one previous)?

I'll see if I can check before using FocalPointPicker.

@timmyc
Copy link
Contributor

timmyc commented Apr 30, 2019

Cool, well i'll keep my test env as-is right now to verify things will still work without 5.2 - and then can install 5.2 and test again.

@ryelle
Copy link
Member Author

ryelle commented Apr 30, 2019

@timmyc I've added a fix, but for some reason FocalPointPicker is not working for me anymore– it keeps defaulting the y value to 100% (even on the cover block, even with this plugin deactivated…) so can you let me know if this tests well for you? (and I'll just keep investigating my setup :/ )

@timmyc
Copy link
Contributor

timmyc commented Apr 30, 2019

Confirmed the changes in 0e19076 have solved the JS exception I was seeing - and I can add a featured product block on a site running 5.1.1 without gutenberg installed. When saving the post with a featured product block, i'm seeing the following on the front-end:

image

FWIW I don't see that when viewing the same page on master.

@ryelle ryelle force-pushed the update/block-featured-product-focalpoint branch from 0e19076 to 5ca1d1c Compare May 2, 2019 15:51
@ryelle
Copy link
Member Author

ryelle commented May 2, 2019

Interestingly I can't reproduce that– but I've switched the check to confirm it's an array (it should always be set from the default in wp_parse_args).

Copy link
Contributor

@timmyc timmyc left a comment

Choose a reason for hiding this comment

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

Confirmed the warning I saw on the front-end is no longer happening, and that all is well when FocalPointPicker is not present, and that it functions when available ( via Gutenberg plugin ) - very cool addition!

@ryelle ryelle merged commit e361dc2 into master May 3, 2019
@ryelle ryelle deleted the update/block-featured-product-focalpoint branch May 3, 2019 14:37
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.

Add FocalPointPicker component to Featured Product block
2 participants