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

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

Open
jsnajdr opened this issue Oct 12, 2023 · 7 comments
Assignees
Labels
[Block] Social Affects the Social Block - used to display Social Media accounts Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Bug An existing feature does not function as intended

Comments

@jsnajdr
Copy link
Member

jsnajdr commented Oct 12, 2023

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.

@jsnajdr jsnajdr added [Type] Bug An existing feature does not function as intended [Block] Social Affects the Social Block - used to display Social Media accounts labels Oct 12, 2023
@cbravobernal
Copy link
Contributor

cbravobernal commented Oct 24, 2023

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

@cbravobernal cbravobernal self-assigned this Oct 24, 2023
@cbravobernal cbravobernal added the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Oct 24, 2023
@colinduwe
Copy link
Contributor

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
Copy link
Contributor

colinduwe commented Dec 8, 2023

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
Copy link
Contributor

I wonder if #56259 (comment) changes the conversation here in any way. Perhaps we can remove the legacy placeholder indicators entirely?

@colinduwe
Copy link
Contributor

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
Copy link

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

@colinduwe
Copy link
Contributor

@jsnajdr @cbravobernal @jasmussen Would one of you be willing to look over #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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Social Affects the Social Block - used to display Social Media accounts Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

5 participants