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

Independent Publister 2: Add spacing between button blocks #8516

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

sixhours
Copy link
Contributor

@sixhours sixhours commented Dec 11, 2024

Changes proposed in this Pull Request:

  • Add a left margin to button blocks that directly follow other button blocks to prevent overlap.
  • Also adds the corresponding RTL styles.

Before

Screenshot 2024-12-11 at 1 14 52 PM

After

Screenshot 2024-12-11 at 3 23 21 PM

Related issue(s):

Fixes #8511

Copy link
Contributor

Preview changes

I've detected changes to the following themes in this PR: Independent Publisher 2.
You can preview these changes by following the links below:

I will update this comment with the latest preview links as you push more changes to this PR.

Note

The preview sites are created using WordPress Playground. You can add content, edit settings, and test the themes as you would on a real site, but please note that changes are not saved between sessions.

@sixhours
Copy link
Contributor Author

There's something unique to WordPress.com that's causing this behavior, because when I apply this patch to a different test site, I get extra spacing between the buttons:

Screenshot 2024-12-11 at 2 01 44 PM

The markup hasn't changed. I'll keep digging and see what the difference is.

@sixhours
Copy link
Contributor Author

Not sure what the difference is, but updating button styles to border-box and updating the padding solves this problem on WP.com without making the spacing wonky on self-hosted installations.

Should I worry about the Theme Check results? Do we usually make updates to older themes to comply or is it safe to let those slide?

@sixhours sixhours force-pushed the fix/ip2-button-spacing branch from 85a8423 to 824e454 Compare January 8, 2025 19:03
Copy link
Contributor

github-actions bot commented Jan 8, 2025

Theme-Check results

independent-publisher-2: There are required changes on the theme ❌.

❎ REQUIRED

  • Could not find wp_body_open action or function call at the very top of the body just after the opening body tag. See: wp_body_open
  • Tested up to: is missing from your style.css header. Also, this should be numbers only, so 5.0 and not WP 5.0
  • Requires PHP: is missing from your style.css header.
💡 RECOMMENDED (3)
  • No reference to register_block_pattern was found in the theme. Theme authors are encouraged to implement custom block patterns as a transition to block themes.
  • No reference to register_block_style was found in the theme. Theme authors are encouraged to implement new block styles as a transition to block themes.
  • No reference to add_editor_style() was found in the theme. It is recommended that the theme implement editor styling, so as to make the editor content match the resulting post output in the theme, for a better user experience.
⚠️ WARNING (6)
  • Found a translation function that is missing a text-domain in the file inc/customizer.php. Function esc_html_x, with the arguments 'Enter the email address associated with your %1$s, or Globally Recognized Avatar, to be displayed in the header.', 'independent-publisher-2'. Line 30: 'description' => sprintf( esc_html_x( 'Enter the email address associated with your %1$s, or Globally Recognized
  • Found a translation function that is missing a text-domain in the file inc/wpcom-colors.php. Function __, with the arguments 'Headings'. Line 34: ), __( 'Headings' ) );
  • Found a translation function that is missing a text-domain in the file inc/wpcom-colors.php. Function __, with the arguments 'Links'. Line 79: __( 'Links' ) );
  • Found a translation function that is missing a text-domain in the file inc/wpcom-colors.php. Function __, with the arguments 'Buttons'. Line 98: __( 'Buttons' ) );
  • Found a translation function that has an incorrect number of arguments in the file template-parts/content-single.php. Function esc_html_e, with the arguments 'Published', post published, 'independent-publisher-2'. Line 46:
  • More than one text-domain is being used in this theme. This means the theme will not be compatible with WordPress.org language packs. The domains found are independent-publisher-2, post published.

@alaczek
Copy link
Contributor

alaczek commented Jan 8, 2025

Do we usually make updates to older themes to comply or is it safe to let those slide?

Let is slide in this case, I don't think we ever pushed this theme to .org repo. Anyway, looks good to me!

@alaczek alaczek merged commit 0738049 into trunk Jan 8, 2025
1 of 2 checks passed
@alaczek alaczek deleted the fix/ip2-button-spacing branch January 8, 2025 23:03
@alaczek
Copy link
Contributor

alaczek commented Jan 8, 2025

I merged this as it solves the issue on the front end, but in the editor the buttons are still overlapping (both on self-hosted and .com sites):

Editor view:
image

I believe the same fix needs to be applied here:
https://github.com/Automattic/themes/blob/trunk/independent-publisher-2/css/editor-blocks.css#L543

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Independent Publisher 2: Buttons block are overlapping
2 participants