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

Fix: Extra top and bottom margin issue in Social Link block for classic theme below twenty twenty #69100

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

singhakanshu00
Copy link
Contributor

What?

fixes #69098

Why?

  • Extra margin top and bottom is getting added coming from classic.css file.

How?

  • Added margin: 0 to editor styles for Social Link Block.

Testing Instructions

  • Activate any theme below twenty twenty, this seem problem for the theme Twenty Ten to Twenty Twenty
  • Type / to choose a block
  • Select Social Icons block
  • Add number of social icons and its link
  • Now, we can see the extra top & bottom margin into the icons on the editor side.

Screenshots or screencast

Before:
Screenshot 2025-02-07 at 10 53 55 PM

After:
Screenshot 2025-02-07 at 10 54 15 PM

@singhakanshu00 singhakanshu00 marked this pull request as ready for review February 7, 2025 17:42
Copy link

github-actions bot commented Feb 7, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: singhakanshu00 <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: carolinan <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: viralsampat-multidots <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@Mamaduka Mamaduka added [Type] Bug An existing feature does not function as intended [Block] Social Affects the Social Block - used to display Social Media accounts labels Feb 9, 2025
@Mamaduka
Copy link
Member

Mamaduka commented Feb 9, 2025

@carolinan, @luminuu, do you think this needs to be fixed on the theme side? Fixing every theme conflict doesn't seem maintainable long-term, but I would defer to your expertise.

@carolinan
Copy link
Contributor

It is possible that it is a theme issue, like with the pull quote issue, it needs testing first.

@t-hamano
Copy link
Contributor

From what I've researched, it looks like this issue needs to be addressed in the social icons block itself.

See #69098 (comment).

@singhakanshu00
Copy link
Contributor Author

Yes @t-hamano, so adding the margin: 0 to the button does the task. I don't know the idea behind changing the useBlockProps from li to button. If we can't reverse that, this will work as I have tested for the block themes as well ( >= twentytwentyone), and it doesn't causes any regression issue.

@t-hamano
Copy link
Contributor

I don't know the idea behind changing the useBlockProps from li to button.

I think this is for accessibility reasons, see #64883. So it looks like we can't put the useBlockProps position back on the li element.

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 [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Social Icons having top and bottom margin issue in the editor with classic themes
4 participants