search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.52k stars 4.21k forks source link

Social Icons: placeholder styles are broken, don't correspond to the real thing #55296

Open jsnajdr opened 1 year ago

jsnajdr commented 1 year ago

Steps to reproduce:

  1. Create an empty Social Icons block, and let it display as a placeholder with three colored icons.
  2. Next to it create another Social Icons block, and populate it with real social icons.

The result will look like this:

Screenshot 2023-10-12 at 11 35 22

The placeholder icons are not rounded, and there is no space between them. Real icons are inside a flex layout with a gap: 1.5rem CSS property that creates the spacing.

Another instance of broken placeholder styles is when you change the Social Icons styles to have the "Logos Only" style and a black icon color:

Screenshot 2023-10-12 at 11 42 36

Here again the gap is missing, and also:

  1. The icon sizes don't match: the placeholders are 24px, but the real SVGs are 30px big. Although the SVG's size is determined by its font-size: 24px CSS style, somehow that translates into 30px real size.
  2. The black icon color is not respected by the placeholder.

Last time the social icons placeholders were updated was 3 years ago by @jasmussen in #26953. It seems that the real icons' styling has changed since, but the placeholders were left behind.

cbravobernal commented 1 year ago

I will work on fixing this issue during the Core Table at WordCamp Madrid Contributor's Day. (5th of November)

colinduwe commented 11 months ago

I added a PR for the minor style updates. However, when the block is incomplete (showing the placeholder) it does not get the color attribute so we can't display the placeholders in the selected color.

I'm not sure that's the ideal behavior. If you set it to black you want to see three black squares? Happy to investigate further to see if we can make that happen but I suspect that someone had a good reason before going to the trouble to keep that color class off the incomplete block.

colinduwe commented 11 months ago

My apologies. I was rushing at the end of the day. I added classes and styles to the placeholders so they respond to the color controls. The placeholders do not respond to the spacing controls (Block spacing is applied to the container block and the placeholders are contained within a single list item child of that container)

jasmussen commented 11 months ago

I wonder if https://github.com/WordPress/gutenberg/issues/56259#issuecomment-1841861841 changes the conversation here in any way. Perhaps we can remove the legacy placeholder indicators entirely?

colinduwe commented 11 months ago

The polish in that issue is a definite improvement but if it's still possible to remove all the social link blocks from a social links block I think the placeholder is preferable to an empty block container in the editor.

Dusky-Div commented 2 months ago

Is this issue resolved? Because I can't see any PS relating to this issue. Please update!

colinduwe commented 2 months ago

@jsnajdr @cbravobernal @jasmussen Would one of you be willing to look over https://github.com/WordPress/gutenberg/pull/56887 and decide if it's an improvement. If so, I'm happy to revise as needed to get it merged. Or if other work on this block has made that PR and this issue irrelevant, can you close both?