Subscribers: Add success notice for deleted subscriber

[allilevine]

Related to https://github.com/Automattic/loop/issues/488

Proposed Changes

• Add success notice when a subscriber is successfully deleted.

Multiple subscribers deleted:

Note we've decided not to fix the original aria-hidden console warning. Adding focus management made the experience worse. The issue will need to be fixed upstream in core.

Why are these changes being made?

• Improve the experience.

Testing Instructions

• Go to /subscribers/{site url}
• Click on a subscriber to open the details panel, or click the Actions three dots and click Remove.
• Click to delete the subscriber.
• Check that you see a success notice for 5 seconds.

Pre-merge Checklist

• Has the general commit checklist been followed? (PCYsg-hS-p2)
• Have you written new tests for your changes?
• Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
• Have you checked for TypeScript, React or other console errors?
• Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
• Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
  • For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
• For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

[github-actions]

Calypso Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-138164

Jetpack Cloud live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-138164&env=jetpack

Automattic for Agencies live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-138164&env=a8c-for-agencies

[matticbot]

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~132 bytes added 📈 [gzipped])

Details

name         parsed_size           gzip_size
subscribers       +464 B  (+0.0%)     +132 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~140 bytes added 📈 [gzipped])

Details

name                                                 parsed_size           gzip_size
async-load-calypso-my-sites-stats-pages-subscribers       +464 B  (+0.3%)     +140 B  (+0.2%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

[matticbot]

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

• notifications
• odyssey-stats
• wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug fix/subscribers-aria-hidden-warning on your sandbox.

[allilevine]

The only change here is adding the ref.

[dcalhoun]

Thank you for exploring a fix for this! 🙇🏻‍♂️

I agree with the spirit of these changes, but, as noted in an inline comment, I'm uncertain if the new UX is better than the previous version. I'm curious to hear your thoughts on my findings.

---

Unrelated to the proposed changes: I was able to get site's subscribers list into some really odd states—where the total count did not match the subscribers list and I often could not remove subscribers because they supposedly didn't exist. I'm not sure if it was the nature of the emails I used (e.g., name+test1@domain.com, name+test2@domain.com) or the speed at which I was adding/removing subscribers.

Refreshing the page would only sporadically fix the issue. Generally, waiting a while and adding an additional subscriber would cause some (not all) of the previous subscribers to show up in the list.

Subscribers mismatch screenshot

[dcalhoun]

From my testing, I do not hear this message via screen reader—both with or without this logic. That isn't too surprising to me, as screen readers can be quite selective as to what they read aloud depending on what else is going on at the same time. It is quite possible updates to the surrounding UI—e.g., the DataViews—is competing for attention. Do you consistently here this read aloud for screen readers?

I'll note the WordPress Notice component already announces the contained text. In theory, this logic should not be necessary.

[allilevine]

Do you consistently here this read aloud for screen readers?

I did consistently hear this read testing with Brave + VoiceOver. But I can remove it in favor of the Notice. Thanks!

[dcalhoun]

Ideally, we shouldn't need to handle focus ourselves, as the WordPress Modal component provides focusOnMount and automatically hides content outside the modal. However, I can reproduce the reported console error—where the root id="wpcom" container is not hidden from screen readers—when testing the Gutenberg Storybook, which is unfortunate.

From my testing, the proposed focus logic may cause problems more disruptive than the original problem. Namely, I note this new logic:

1. Prevents moving focus to the modal close (X) button;
2. Prevents text selection within the modal;
3. And fails to move focus back to the previous focused element on modal dismissal.

I'm not confident resolving the original console error (and the erroneously visible non-modal content) is worth introducing these new problems. I think the current production version is more accessible.

WDYT?

[holdercp]

I'm also finding that the new implementation doesn't capture focus when Tabbing through:

Screen.Recording.2025-03-14.at.8.16.44.AM.mov

I agree that I don't think this tradeoff is worth fixing the console error. And it seems that @dcalhoun found that the error might need to fixed upstream in the Modal component.

[allilevine]

I'm not confident resolving the original console error (and the erroneously visible non-modal content) is worth introducing these new problems. I think the current production version is more accessible.

Thank you for testing! 🙌 In that case, I'll revert the focus management and we'll need to live with it as is until this is fixed upstream.

[allilevine]

Unrelated to the proposed changes: I was able to get site's subscribers list into some really odd states—where the total count did not match the subscribers list and I often could not remove subscribers because they supposedly didn't exist.

@dcalhoun Thanks for raising this, we have an issue here: https://github.com/Automattic/loop/issues/486 I've added this report.

[holdercp]

Works well.

[a8ci18n]

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/17303543

Some locales (Hebrew) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday.

Thank you @allilevine for including a screenshot in the description! This is really helpful for our translators.

[dlind1]

These strings:

'%s has been removed from your subscribers list.',
'%d subscribers have been removed from your list.',

cause some issues in Korean where there is no explicit plural form and the plurality is inferred from the count. A better solution would be to use something like this pseudocode:

if (count == 1) {
`%s has been removed from your subscribers list.`
} else {
'%d subscriber have been removed from your list.' / '%d subscribers have been removed from your list.'
}

which would take care of the singular subscriber case and cover all the possible plural forms.

[allilevine]

A better solution would be to use something like this pseudocode:

@dlind1 I'm confused since I got the opposite feedback here: #99177 (comment)

Should the pseudocode approach be used when there are different variables? Or when the strings are different enough?

[dlind1]

@dlind1 I'm confused since I got the opposite feedback here: #99177 (comment)

Should the pseudocode approach be used when there are different variables? Or when the strings are different enough?

The feedback in #99177 is correct. When one string refers to a quantity and could contain singular/plural nouns, like %d subscriber / %d subscribers, a plural translate call should be used. When you want to display a different string for a specific quantity/case, the best approach would be the one in the pseudocode - Subscriber named %s for a single subscriber mentioned by name and a plural string %d subscriber / %d subscribers for all the countable cases.

[dlind1]

To add to that, when using translations with plurals the same placeholders should be used in the singular and the plural forms 99% of the time, with maybe a couple exceptions.

[allilevine]

@dlind1 Thanks for explaining! 🙌 I've updated the strings:

wp-calypso/client/my-sites/subscribers/hooks/use-unsubscribe-modal.ts

Lines 51 to 67 in 0a9cda8

	subscribers.length === 1
	? translate( '%(name)s has been removed from your subscribers list.', {
	args: {
	name: subscribers[ 0 ].display_name,
	},
	comment: 'Shows when a single subscriber is removed, using their name',
	} )
	: translate(
	'%(count)d subscriber has been removed from your list.',
	'%(count)d subscribers have been removed from your list.',
	{
	count: subscribers.length,
	args: {
	count: numberFormat( subscribers.length ),
	},
	comment: 'Shows when multiple subscribers are removed, using the count',
	}

Let me know if more changes are needed.

[dlind1]

@dlind1 Thanks for explaining! 🙌 I've updated the strings:

Looks great, thank you! 🎉