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 Links has inconsistent spacing when child of Navigation #37935

Closed
tellthemachines opened this issue Jan 13, 2022 · 9 comments
Closed

Social Links has inconsistent spacing when child of Navigation #37935

tellthemachines opened this issue Jan 13, 2022 · 9 comments
Labels
[Block] Navigation Affects the Navigation Block [Block] Social Affects the Social Block - used to display Social Media accounts [Type] Bug An existing feature does not function as intended

Comments

@tellthemachines
Copy link
Contributor

Description

When the Social Links block is a child of the Navigation block, it inherits the gap spacing from Navigation on the front end, but not in the editor.

This seems to happen because Social Links default styles take the gap value from --wp--style--block-gap variable. If the parent Navigation has custom spacing set, it outputs a new value for --wp--style--block-gap on the front end, but not in the editor.

I'm not sure if it's desirable to have Social Links follow the spacing rules of its parent or not, so not sure what the best fix for this would be, but it should at least be consistent between editor and front end.

Step-by-step reproduction instructions

  1. Activate either Twenty Twenty Two or Empty Theme
  2. Create a Navigation block with a few links and a Social Links block inside it (with at least two socials, so we can see the spacing between them).
  3. In the Nav block sidebar, under "Dimensions", set "Block spacing" to a custom value.
  4. Save and compare results between editor and front end.

Screenshots, screen recording, code snippet

Editor:
Screen Shot 2022-01-13 at 4 11 07 pm

Front end:

Screen Shot 2022-01-13 at 4 11 15 pm

Environment info

Latest Gutenberg trunk.

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@tellthemachines tellthemachines added [Type] Bug An existing feature does not function as intended [Block] Navigation Affects the Navigation Block [Block] Social Affects the Social Block - used to display Social Media accounts labels Jan 13, 2022
@ramonjd
Copy link
Member

ramonjd commented Jan 13, 2022

I'm not sure if it's desirable to have Social Links follow the spacing rules of its parent or not, so not sure what the best fix for this would be, but it should at least be consistent between editor and front end.

Social links already opts-in to blockGap support so maybe it's okay for the block to take care of its own gap values?

I tried replicating this bug but couldn't as much as I tried! Here's me in TT2:

Jan-13-2022.20-10-19.mp4

Am I doing it right?

I see the default gap value is --wp--style--block-gap for both editor and frontend, and the custom gap value is used when I set it.

@tellthemachines
Copy link
Contributor Author

@ramonjd if you set a gap value for Social Icons, the issue won't be noticeable because that value will override the one inherited from Nav. Can you not reproduce it even if Social Icons has no gap set?

If you can be bothered, another option is to try empty theme from this repo to make sure there's no theme style contamination 😅

@ramonjd
Copy link
Member

ramonjd commented Jan 14, 2022

Can you not reproduce it even if Social Icons has no gap set?

Yeah, in the first half of the recording I didn't set the Social Icons gap and I couldn't reproduce.

I did try empty theme as well, but often things happen to me only, so I'll definitely try again 😄

@ramonjd
Copy link
Member

ramonjd commented Jan 14, 2022

Okay, I see it now!

social-links-in-a-nav.mp4

Not sure yet, but my first guess is because we're not ignoring blockGap in rendered blocks as we do for the js hook.

Looking at fixing it in navigation/index.php for now, but it might required a more high level fix. 👀

@ramonjd
Copy link
Member

ramonjd commented Jan 14, 2022

The variable between my first test (in which I could not reproduce) and the second test was that I was using WordPress 5.8.2.

When using 5.9 beta (basically wp-env from the Gutenberg trunk) I can reproduce. 🤔

I can reproduce this on

WordPress: 5.9-beta3-52383
Gutenberg: 12.4.0-rc.1

@ramonjd
Copy link
Member

ramonjd commented Jan 14, 2022

The plot thickens

I checked out https://github.com/WordPress/wordpress-develop and built the Gutenberg trunk plugin, and can longer reproduce 😭

WordPress: 6.0-alpha-52448-src
Gutenberg: 12.4.0-rc.1

Screen Shot 2022-01-14 at 1 56 18 pm

@ramonjd
Copy link
Member

ramonjd commented Jan 14, 2022

I also cannot reproduce on:

WordPress: 5.9-RC2-52567-src
Gutenberg: 12.4.0-rc.1

@ramonjd
Copy link
Member

ramonjd commented Jan 14, 2022

This PR removed the gap inline style from block wrappers on the frontend and backend #37360

It was added to WP in this changeset 10 days ago https://core.trac.wordpress.org/changeset/52434/

Given that it works in a 5.9 RC, I think we should be okay?

@tellthemachines
Copy link
Contributor Author

I can no longer reproduce the issue either when running latest Gutenberg on latest WP trunk. Closing, thanks for all the testing @ramonjd !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [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

No branches or pull requests

2 participants