Refactor the Media&text block to use the new color support flag #21169
Conversation
youknowriad
requested review from
ajitbohra,
jorgefilipecosta,
Soean and
talldan
as code owners
|
Size Change: +144 B (0%) Total Size: 856 kB
ℹ️ View Unchanged
|
|
This is pretty cool, nice! A couple of small things/questions that may or may not be related to this branch. Screenshot first:
I can't use a custom color at all. It just reverts to default when I try and pick one. Text and BG colors both. This panel starts collapsed by default, shouldn't it be open?
This thing at the bottom where the background bleeds to the left annoys me. It looks like if you apply This is with the image set to flex:
|
I forgot to add the CSS variables :P |
That's something that applies across blocks. If we want it, happy to change it. |
It seems unrelated with this PR right? |
|
Is the feature global across blocks yet or no? I'd say worth doing it once these properties become global. |
Yes, but it is made more visible by the fact that you can now apply a background color, so it should be addressed. Whether in this PR or a separate one right after is totally fine, but important to not do one without the other, or we'll ship a subpar background experience. |
The feature is now used in columns and group and there are PRs for heading and paragraph |
Agreed, the block already supports background colors though. @jorgefilipecosta would be a better person to know a fix that doesn't impact the behavior of the block. (Also maybe it should apply to videos too) |
| @@ -6,12 +6,6 @@ | |||
| "type": "string", | |||
| "default": "wide" | |||
| }, | |||
| "backgroundColor": { | |||
There was a problem hiding this comment.
It's great to see all this disappearing :)
youknowriad
force-pushed
the
update/media-text-color-hook
branch
from
89632ff
to
8673e1a
Compare
| .wp-block-media-text > figure > img, | ||
| .wp-block-media-text > figure > video { | ||
| .wp-block-media-text__media img, | ||
| .wp-block-media-text__media video { |
There was a problem hiding this comment.
@jasmussen looks like the styles to handle the image height properly were already present but not applied in the editor because of an extra div. This should fix it
|
Can I have a ✅ here :) |
Thank you for the heads up I will look into a fix for this. |
|
@jorgefilipecosta I actually already pushed a fix here 4308d34 |
This also adds support for text color support at the same time. We can add config to disable it but I thought it was a good idea to leave it for consistency.