Cover: Video - Add position controls #22531
Conversation
Improve grid CSS rendering.
ItsJonQ
added
[Type] Enhancement
| * Internal dependencies | ||
| */ | ||
| import { color } from '../../utils/style-mixins'; | ||
|
|
There was a problem hiding this comment.
Migrated and improved styles from the previous styles.scss file.
|
Size Change: +1.41 kB (0%) Total Size: 1.14 MB
ℹ️ View Unchanged
|
|
@jasmussen Provided a design suggestion in the issue. Will attempt it now 🙌
|
|
@jasmussen We have "side" label alignments for Inputs :D
Note! This is done at the |
|
@jasmussen Based on your mockup, do you think we should remove the extra padding around the media (when possible)?
Doing so would change the height of the preview. We could set min/max values to ensure it doesn't get too wonky. It comes down to... do we want the preview to have a consistent height (currently Would love your thoughts! |
|
@jasmussen Amazing! Thank you <3 |
|
Update! Re: Code Review I intentionally left the |
packages/components/src/focal-point-picker/styles/focal-point-picker-style.js
Show resolved
Hide resolved
| export default function FocalPoint( { | ||
| coordinates = { left: '50%', top: '50%' }, | ||
| isDragging = false, | ||
| ...props |
There was a problem hiding this comment.
I noticed you do that often (spread extra props) and I'm having trouble figuring out why or whether I consider this a good or bad practice. Are these needed? I know I personally try to avoid them if possible as they can lead to react warnings and can be hard to follow.
There was a problem hiding this comment.
For component libraries, prop spreading is common and often the default. The reason is because the consumer should be able to use HTML approved properties (like aria-* or data-*) just like they would on any HTML element. Not only that, but on* event callbacks should be available as well.
A (mostly) reliable solution for handling React warnings is that inner components often have prop filtering built in. In the case of components created with @emotion/styled, they filter out non React/HTML props before passing it down to elements like <div />, <input />, etc...
There was a problem hiding this comment.
For component libraries, prop spreading is common and often the default. The reason is because the consumer should be able to use HTML approved properties (like aria-* or data-) just like they would on any HTML element. Not only that, but on event callbacks should be available as well.
When I use FocalPointPicker as a user, I don't know its internals whether it's a div, a button, or just a string... which makes me think, I don't know what HTML prop are approved. This also makes it harder to be compatible with mobile. A block doing < FocalPointPicker some-dom-specific-prop="something" /> won't translate properly to mobile.
It makes me wonder if we should try to move away from these iteratively.
packages/e2e-tests/fixtures/blocks/core__cover__deprecated-4.serialized.html
Outdated
Show resolved
Hide resolved
|
@youknowriad Thank you for reviewing this @youknowriad + @jasmussen ! I'll merge it up 🙌 |
|
Looks like this PR might have introduced a JavaScript unit test error on master 😬 |
|
@youknowriad Uh oh!! Will look into that now |
|
@youknowriad PR incoming with fix~! |
This update adds positioning controls for video backgrounds within the Cover block. Previous to this image, the positioning controls (powered by the
FocalPointPickercomponent) were available only to image backgrounds.✨ New Features
Click to reposition
The position values can be updated by clicking anywhere in the preview area.
Keyboard arrow key support
Up, Down, Left, Right keyboard keys will adjust the position values.
Hold Shift to jump by 10
Holding Shift will jump the values, rounded to the nearest
10. This works for boht dragging and keyboard arrow inputs.New InputControls
The
topandleftvalues use a newNumberControlcomponent. This component includes many features, such as dragging to update, jumping values by holding Shift, and automatic number rounding.Grid
A 9x9 grid appears anytime a value is updated by any interaction (drag, click, etc...)
🛠 Fixes
<FocalPointPicker />to reset values based on undo actions in the editor.How has this been tested?
This has been tested locally in Storybook and local Gutenberg.
To test...
npm installnpm run devto fire up local GutenbergTypes of changes
FocalPointPickerFocalPointPickerFlexcomponent to@wordpress/componentsas a layout-based primitiveedit.jsandsave.jsChecklist:
Resolves: #22509