feat[my-account-block]: add option to toggle off icon/text, and update tests by laurelfulford · Pull Request #4443 · Automattic/newspack-plugin

laurelfulford · 2026-02-02T18:57:28Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This PR adds an option to hide either the icon or label on the My Account Button block (but not both at the same time).

As part of that, I changed how the button acts if you don't give it a text label (instead of not showing anything, it will fallback to the default labels) -- when it's hidden in the settings, the label is still used as screen reader text so we don't want it to be empty even if it's not visible.

I also updated the tests to check for the default (vs. making sure empty strings were empty), and tweaked the behaviour to make sure logged in admins (not just readers) would see the My Account labels - I thought this was working before but I was seeing Sign In in trunk.

See NPPD-1183.

How to test the changes in this Pull Request:

Apply this PR and run npm run build. As a bonus you can also test this alongside feat: make My Account button only show icon on mobile newspack-block-theme#406, but it's not required.
If using the default header patterns, the button should already be in the header. If not, add the My Account Button block to the header.
Click on the button and confirm you have a Display panel under the Settings tab, with an option to Display label and Display icon. Both should be enabled by default:

Try turning of each option and confirm that the button updates appropriately (hiding the text or the icon). Confirm that you can't turn off both options at once -- if you try to turn off say, the icon, the label should switch to visible if it isn't already, and vis versa.
Try changing the text labels on each state of the button (Signed in, Signed out), turn off the Show label toggle, and view on the front end.
Confirm you only see the icons; when you inspect the icon, you should see your label text wrapped in a screen-reader-text span.
Try signing in/signing up in an incognito window, and confirm hidden text is right (that you can see the Signed out text in the markup when you're not logged in, and Signed in text when you are).
Try editing the header again and switching the settings so you can see the text, but not the icon; repeat step 7.
Try editing the text label of each button and leave it blank; confirm you can still see the placeholder in the editor.
Publish your changes and confirm you see the default text on the front-end (Sign In and My Account).
Run the tests just to make sure they come back okay.

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully ran tests with your changes locally?

Copilot

Pull request overview

This PR adds display controls to the My Account Button block, allowing users to selectively hide either the icon or label text, while ensuring at least one element remains visible for usability. The PR also improves the empty label fallback behavior and fixes an issue where logged-in admins were seeing incorrect button labels.

Changes:

Added showLabel and showIcon boolean attributes with UI toggles in the Inspector Controls
Updated empty label handling to fallback to defaults instead of rendering nothing
Added data-wp-logged-in attribute to preserve correct labels for logged-in WordPress users
Updated tests to verify fallback behavior for empty labels

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/blocks/my-account-button/block.json	Added showLabel and showIcon boolean attributes with default values of true
src/blocks/my-account-button/edit.js	Implemented Inspector Controls with toggle controls and mutual exclusivity logic, added conditional rendering for icon/label visibility
src/blocks/my-account-button/class-my-account-button-block.php	Added showLabel/showIcon attribute handling, conditional rendering logic, screen-reader-text class for hidden labels, and data-wp-logged-in attribute
src/reader-activation-auth/index.js	Added check for logged-in WordPress users to preserve correct label text and prevent unnecessary reader authentication checks
tests/unit-tests/class-test-my-account-button-block.php	Split empty label test into separate signed-in/signed-out tests, updated assertions to verify fallback to default labels
src/blocks/my-account-button/README.md	Added documentation for the new Display toggle feature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/blocks/my-account-button/class-my-account-button-block.php

tests/unit-tests/class-test-my-account-button-block.php

thomasguillot · 2026-02-03T18:07:25Z

Here's an update, using styles: d315db9

laurelfulford · 2026-02-03T18:14:27Z

@thomasguillot Looks good to me! 🙂 Now we just need to see what an uninvolved third-party has to say!

rbcorrales

Nice approach using block styles instead of custom toggles! Left a few inline comments, the main ones being the double useBlockProps() call in the editor and the data-wp-* namespace on the logged-in attribute. The rest is mostly minor.

rbcorrales · 2026-02-10T18:41:47Z

src/blocks/my-account-button/edit.js

 	const spacingProps = useSpacingProps( attributes );
+
+	// Block style comes from the Styles panel (wrapper) or saved className.
+	const blockWrapperProps = useBlockProps();


Calling useBlockProps() twice in the same component is not a supported pattern. Each call generates its own ref and block binding, so the editor will bind internal state (selection, focus, drag) to whichever ref was registered last. The first call here is never attached to a DOM node, which can cause subtle bugs.

The is-style-* class is already available on attributes.className (already serialized by WP). You can technically derive the style variant directly from blockClassName without this extra call:

const isIconOnly = ( blockClassName || '' ).includes( 'is-style-icon-only' ); const isTextOnly = ( blockClassName || '' ).includes( 'is-style-text-only' );

rbcorrales · 2026-02-10T18:41:47Z

src/blocks/my-account-button/class-my-account-button-block.php

+		$wrapper_attribute_args = [
+			'class'             => implode( ' ', $extra_classes ),
+			'href'              => \esc_url_raw( $href ),
+			'data-wp-logged-in' => $is_signed_in ? '1' : '0',


The data-wp-* prefix is used by WordPress Interactivity API, and custom plugins should avoid it to prevent future collisions. Can we rename it to something different, like data-newspack-logged-in?

Also, this attribute explicitly states the user's authentication state in the DOM, though it's already inferable from the button's href (hashtag vs anything else). Maybe this is not really needed.

Thanks @rbcorrales! I updated the attribute name in 81ee30d, but left it -- it's a little yucky, but the My Account Button block behaves a little different than the My Account link that's inserted into the theme (the latter isn't visible for logged in non-readers, and when it is in the Customizer, it says 'Sign In'). The attribute seemed like the nicest way to switch it in just the one spot.

rbcorrales · 2026-02-10T18:41:47Z

src/blocks/my-account-button/class-my-account-button-block.php

+
+		/** Display mode from block style class in className (default = icon + text). */
+		$wrapper_class = (string) $attrs['className'];
+		if ( \strpos( $wrapper_class, 'is-style-icon-only' ) !== false ) {


A hypothetical additional class like is-style-icon-only-alt would also match. A more robust approach:

$classes = explode( ' ', (string) $attrs['className'] ); if ( in_array( 'is-style-icon-only', $classes, true ) ) {

Good call! Fixed in a49f0e0.

rbcorrales · 2026-02-10T18:41:47Z

src/blocks/my-account-button/edit.js

-				</ToolbarGroup>
-			</BlockControls>
-			<div className={ classnames( 'wp-block-buttons', customClassName ) }>
+			{ isLabelVisible && (


When icon-only style is selected, isLabelVisible is false, so the "Signed out"/"Signed in" toolbar toggle disappears entirely. The user cannot edit the screen reader label text for the signed-in state without switching styles first, and then back.
I'm not sure if there's a solution for this, since it needs to render the text in order to be modified, or if this is even an issue 🤔.

Yeah but I think it's OK since it uses the default aria-labels

It is a little funky, but I think it's probably okay as is for now -- or at least the kind of thing that should be covered in the documentation (how to edit the hidden label), and if we get a lot of feedback about friction, we can revisit 🙂

Readme has been updated in cf68793!

I'd say keep it as it is for now. If we get feedback about this, we can always add 2 textcontrols to the sidebar

rbcorrales · 2026-02-10T18:41:48Z

src/blocks/my-account-button/class-my-account-button-block.php

+		$signed_out_label = '' === trim( (string) $attrs['signedOutLabel'] ) ? $default_attrs['signedOutLabel'] : $attrs['signedOutLabel'];
+		$label            = $is_signed_in ? $signed_in_label : $signed_out_label;
+
+		/** Display mode from block style class in className (default = icon + text). */


Nit: WPCS recommends // for inline comments. /** */ style is reserved for docblocks.

rbcorrales · 2026-02-10T18:47:53Z

src/blocks/my-account-button/class-my-account-button-block.php

Not on this PR, but $default_block_class on line 148 is assigned but never used.

Removed as part of 9fed074

rbcorrales · 2026-02-10T18:49:51Z

src/blocks/my-account-button/edit.js

+	const isIconOnly = className.includes( 'is-style-icon-only' );
+	const isTextOnly = className.includes( 'is-style-text-only' );


Similar comment about class names matching.

thomasguillot · 2026-02-10T20:36:00Z

@laurelfulford I tried to fix some of the things @rbcorrales mentioned. I haven’t touched the PHP, if you’d like to take care of that.

laurelfulford · 2026-02-10T22:59:51Z

Thanks for the review, @rbcorrales, and for the fixes, @thomasguillot!

I think I got the rest, but let me know if anything still seems funky. I'm going to update the Docs in Linear to also reflect the little "gotcha" around editing the label when hidden.

rbcorrales

Thanks for addressing all the feedback @laurelfulford @thomasguillot! Everything looks good, but one thing that wasn't addressed: the class matching in edit.js still uses String.includes(), which could match partial class names (e.g. is-style-icon-only-alt). The PHP side was correctly updated to explode + in_array, but the JS side wasn't. The equivalent fix:

const classes = ( blockClassName || '' ).split( ' ' );
const isIconOnly = classes.includes( 'is-style-icon-only' );
const isTextOnly = classes.includes( 'is-style-text-only' );

Approving, but would be great to get that last item in before merging.

laurelfulford · 2026-02-11T21:53:28Z

Thanks @rbcorrales!

Everything looks good, but one thing that wasn't addressed: the class matching in edit.js still uses String.includes()...

🤦‍♀️ This was entirely my bad, and is fixed in b47c2a2 -- thank you for catching that!

github-actions · 2026-02-11T22:22:24Z

Hey @laurelfulford, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

feat: add option to toggle off icon/text, and update tests

c25371f

laurelfulford requested a review from a team as a code owner February 2, 2026 18:57

laurelfulford requested review from Copilot and removed request for a team February 2, 2026 18:57

Copilot started reviewing on behalf of laurelfulford February 2, 2026 18:57 View session

laurelfulford mentioned this pull request Feb 2, 2026

feat: make My Account button only show icon on mobile Automattic/newspack-block-theme#406

Merged

6 tasks

Copilot AI reviewed Feb 2, 2026

View reviewed changes

src/blocks/my-account-button/class-my-account-button-block.php Outdated Show resolved Hide resolved

tests/unit-tests/class-test-my-account-button-block.php Show resolved Hide resolved

laurelfulford added 2 commits February 2, 2026 11:07

fix: remove unnecessary check

9d9a68d

feat: add more test coverage for new settings

84a9ef8

laurelfulford added the [Status] Needs Review The issue or pull request needs to be reviewed label Feb 2, 2026

laurelfulford requested a review from a team February 2, 2026 19:28

laurelfulford added the [Status] Needs Design Review label Feb 2, 2026

laurelfulford requested a review from thomasguillot February 2, 2026 19:28

laurelfulford mentioned this pull request Feb 2, 2026

feat: add My Account button block to all header patterns Automattic/newspack-block-theme#404

Merged

6 tasks

fix(my-account-button): apply icon margin if no screen-reader-text

ced713a

thomasguillot force-pushed the feat/my-account-button-display branch from 8d2f75a to 666a61a Compare February 3, 2026 18:02

feat: replace toggles with styles and classnames

d315db9

thomasguillot force-pushed the feat/my-account-button-display branch from 666a61a to d315db9 Compare February 3, 2026 18:04

Automattic deleted a comment from laurelfulford Feb 3, 2026

Merge branch 'trunk' into feat/my-account-button-display

47dba2a

rbcorrales requested changes Feb 10, 2026

View reviewed changes

github-actions bot added the [Status] Needs Changes or Feedback The issue or pull request needs action from the original creator label Feb 10, 2026

thomasguillot added 2 commits February 10, 2026 20:26

chore: update inline comments

9fed074

fix: use blockClassName in favour of useBlockProps

0581242

laurelfulford added 3 commits February 10, 2026 14:45

fix: update attribute used for the My Account button block logged in

81ee30d

fix: improve classname checks

a49f0e0

fix: update readme with label editing behaviour

cf68793

laurelfulford requested a review from rbcorrales February 10, 2026 22:59

laurelfulford removed the [Status] Needs Changes or Feedback The issue or pull request needs action from the original creator label Feb 10, 2026

thomasguillot removed the [Status] Needs Design Review label Feb 11, 2026

thomasguillot removed their request for review February 11, 2026 13:46

thomasguillot force-pushed the feat/my-account-button-display branch from 40a4021 to f07b889 Compare February 11, 2026 13:50

fix: editor alignment

86d6e24

thomasguillot force-pushed the feat/my-account-button-display branch from f07b889 to 86d6e24 Compare February 11, 2026 14:01

Merge branch 'trunk' into feat/my-account-button-display

44f111f

rbcorrales approved these changes Feb 11, 2026

View reviewed changes

github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Feb 11, 2026

fix: harden icon/text-only class name checks

b47c2a2

laurelfulford merged commit 5082b55 into trunk Feb 11, 2026
7 checks passed

laurelfulford deleted the feat/my-account-button-display branch February 11, 2026 22:22

github-actions bot added the needs-changelog label Feb 11, 2026

		const isIconOnly = className.includes( 'is-style-icon-only' );
		const isTextOnly = className.includes( 'is-style-text-only' );

Conversation

laurelfulford commented Feb 2, 2026

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

thomasguillot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laurelfulford commented Feb 3, 2026

Uh oh!

rbcorrales left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomasguillot commented Feb 10, 2026

Uh oh!

laurelfulford commented Feb 10, 2026

Uh oh!

rbcorrales left a comment

Choose a reason for hiding this comment

Uh oh!

laurelfulford commented Feb 11, 2026

Uh oh!

Uh oh!

github-actions bot commented Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thomasguillot commented Feb 3, 2026 •

edited

Loading