Skip to content

Conversation

@laurelfulford
Copy link
Contributor

@laurelfulford laurelfulford commented Jan 20, 2026

All Submissions:

Changes proposed in this Pull Request:

This PR, along with Automattic/newspack-block-theme#400 and Automattic/newspack-theme#2618 adds a My Account button block to the Newspack Plugin, adds the button to just one pattern (for now) in the Block Theme (for testing), and moves some CSS from the Newspack Plugin to the classic theme to avoid clashing.

I have some open questions/possible weird cruft to still deal with:

  • For the block itself, it's a pretty basic button-style block with an editable label and an icon. You can edit most things you can edit with a regular button.
  • It does not have unique state for mobile - I was wondering if that should be something we do based on the screen breakpoint, or something we do in the block settings (eg. have a My Account button block style that shows only the icon that can be used on mobile?) cc @thomasguillot
  • Some of the logic for the block -- specifically the trigger that opens the sign-in modal and the JavaScript that updates the button label based on whether you're a reader -- still lives in the RAS code outside of the block. I think this is okay but I might be wrong!
  • Rather than hiding the block when you're logged in as an admin/not a reader, I gave it a 'disabled' state. I find it kind of confusing that the button isn't visible to admins in the classic theme, and I think that's how we end up with it looking so weird in some publisher headers: they're always logged in so they don't see it.
  • I've only added the block to the default header pattern in the theme to start, and added a check for RAS before it gets inserted. Once I nail down the mobile bit, I can add it to the rest!

Some of these (like the mobile state, and maybe adding an 'Outline' style) can also be handled in a separate PR if there isn't a for-sure direction to follow.

See NPPD-1123.

How to test the changes in this Pull Request:

  1. Start with a test site running RAS.
  2. Apply this PR, feat: Add My Account button block  newspack-block-theme#400 and feat: move some My Account button styles to the theme newspack-theme#2618.
  3. Confirm the default header has the My Account button in the header, alongside the Donate Block (note: if you've edited the Header Desktop template part, you'll need to reset it)
  4. In an incognito window, click the button and confirm it triggers the My Account login window; once you sign up/sign in, the button should switch to say My Account.
  5. In your regular window (as an admin) the button should still be visible but slightly greyed out:
CleanShot 2026-01-20 at 16 28 15
  1. Open the Site Editor, and edit the Header (Desktop) template part.
  2. When you click the My Account button, you should have an option to switch between Signed In and Signed Out states.
CleanShot 2026-01-20 at 16 28 52
  1. In each state, try editing the button text to change it to something else.
  2. Try changing some other block options: colours, font size, border radius, padding...
  3. Save and publish your changes.
  4. Confirm they display on the front end, and that your labels both appear for your incognito reader:
CleanShot 2026-01-20 at 16 33 26@2x
  1. Test these PRs on a site running the Classic theme. It should look/act the same -- I mostly wanted to move the styles so we wouldn't have to be overwriting too much classic-theme specific stuff in the block.
  2. Lastly, if possible test these PRs on a site without RAS and confirm that the My Account button isn't inserted.

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?

@laurelfulford laurelfulford marked this pull request as ready for review January 21, 2026 00:37
@laurelfulford laurelfulford requested a review from a team as a code owner January 21, 2026 00:37
@laurelfulford laurelfulford added [Status] Needs Review The issue or pull request needs to be reviewed [Status] Needs Design Review labels Jan 21, 2026
@laurelfulford laurelfulford requested a review from Copilot January 21, 2026 00:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new "My Account Button" block to Newspack Plugin, allowing sites with Reader Activation enabled to display a dynamic button that links readers to their account page or opens the sign-in modal. The block adapts its label and behavior based on authentication state.

Changes:

  • Added a new My Account Button block with dynamic labels for signed-in and signed-out states
  • Moved CSS styles from reader-activation-auth to allow reuse by the new block
  • Added account icon to the icons package

Reviewed changes

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

Show a summary per file
File Description
src/blocks/my-account-button/class-my-account-button-block.php Server-side block rendering with authentication state handling and WooCommerce account URL detection
src/blocks/my-account-button/edit.js Block editor component with toolbar toggle for previewing signed-in/out states
src/blocks/my-account-button/block.json Block metadata with support for color, typography, spacing, and border customization
src/blocks/my-account-button/style.scss Block styles including disabled state and icon sizing
src/blocks/my-account-button/index.js Block registration with icon configuration
src/blocks/my-account-button/README.md Documentation describing block behavior and settings
src/blocks/index.js Registration logic preventing block from appearing in classic themes
packages/icons/src/account.js New account SVG icon
packages/icons/index.js Export for new account icon
includes/class-blocks.php Include block PHP class and pass block theme flag to editor
src/reader-activation-auth/style.scss Removed account link styles (moved to block-specific styles)

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

@thomasguillot
Copy link
Contributor

thomasguillot commented Jan 21, 2026

The button should not disappear or be disabled. It should then link to My account.

Screenshot 2026-01-21 at 12 25 51@2x

(ignore the change of style in the screenshot above)

@laurelfulford
Copy link
Contributor Author

Thanks @thomasguillot!

The button should not... be disabled. It should then link to My account.

The logic in the RAS code only switches the button to the signed in state when you're a registered reader and not when you're logged in. I can do this differently in the block and just have it show the signed in label for anyone who's logged in, but I didn't know the history of that choice re: making it reader specific. Do you?

@thomasguillot
Copy link
Contributor

The logic in the RAS code only switches the button to the signed in state when you're a registered reader and not when you're logged in

Isn't it the same thing? 😕

@thomasguillot
Copy link
Contributor

@laurelfulford I pushed some changes. Basically instead of having yet another custom button block, I think we should leverage Core's button block markup (note: I haven't looked at the editor and it might be broken)

@laurelfulford
Copy link
Contributor Author

Thanks @thomasguillot!

Basically instead of having yet another custom button block, I think we should leverage Core's button block markup

That's a good call! I'll take a look at the editor...

Isn't it the same thing? 😕

Not quite - it distinguishes between someone who's been registered through reader registration vs. other WordPress roles (like editor or administrator).

Right now the button's hidden if you're logged into the other roles (unless you're in the Customizer) so what it shows to the publisher is kind of moot in the classic theme. We could just make it visible and treat all logged in users the same if that makes more sense to you? I can ask the wider team if this would cause issues for some reason -- I don't remember the decision making behind this.

@laurelfulford
Copy link
Contributor Author

@thomasguillot So far it sounds like the main concern was creating kind of a dead end for admins/editors since they don't need to use /my-account.

What would be your preference for how it behaves for logged in publishers: look/act the same as for readers (say "My Account" and link to /my-account) or do something else that (somehow) indicates it's for readers. I'm thinking like how the Newspack Subscription block doesn't let you subscribe as a admin/editor 🤔

@thomasguillot
Copy link
Contributor

Look/act the same as for readers (say "My Account" and link to /my-account)

This 👍

@laurelfulford
Copy link
Contributor Author

laurelfulford commented Jan 21, 2026

Thanks @thomasguillot! I've made the following changes:

  • I tweaked the editor markup so it matches the front-end structurally and visually
  • I removed the 'disabled' styles from the block
  • I made it so the button block always goes to /my-account when the user is logged in, including editorial roles

One last thing from me: what do you think we should do for mobile? I was thinking either make the button only show the icon on small screens, or have an icon-only style as an option that can be added to the mobile header. Or maybe a third option - do you have any preferences?

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

Labels

[Status] Needs Design Review [Status] Needs Review The issue or pull request needs to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants