Add "Shared with you" badge to Extensions > My Subscriptions list as applicable

[Dan-Q]

Changes proposed in this Pull Request:

Fixes WCCOM#19063.

To make it easier to see when an extension is available because of a shared subscription, this PR amends the Extensions > My Subscriptions page for connected stores to:

• Add a "Shared with you" badge to products available as a result of a shared subscription
  • In a popup, shows the email address of the person who shared the subscription with you and a link to "learn more"
• Removes the "Manage on Woo.com" link from the actions menu for subscriptions shared with you
• Reduces the font size on such badges to 12px, in accordance with the latest designs
• Improves the spacing of these badges, where multiple badges appear on multiple lines, so they don't collide

How to test the changes in this Pull Request:

1. Connect a store using a Woo.com account that has at least one subscription shared with it.
2. In WooCommerce > Extensions > My Subscriptions, observe the new "Shared with you" badge.
3. Click on the badge to see the email address of the person sharing it and a link to the sharing documentation.
4. Note that the font size of the badge is now 12px.
5. Click the kebab menu menu and observe that the "Manage on Woo.com" link does not appear for shared subscriptions.
6. (Optional) Force a second badge to appear, e.g. by manually editing plugins/woocommerce-admin/client/marketplace/components/my-subscriptions/table/rows/functions.tsx and adding another <StatusPopover>; observe that the badges have both horizontal and now vertical margins to prevent them from colliding with one another.

Changelog entry

• Automatically create a changelog entry from the details below.

Significance

• Patch
• Minor
• Major

Type

• Fix - Fixes an existing bug
• Add - Adds functionality
• Update - Update existing functionality
• Dev - Development related task
• Tweak - A minor adjustment to the codebase
• Performance - Address performance issues
• Enhancement - Improvement to existing functionality

Message

For shared subscriptions, My Subscriptions now shows "Shared with you" and the email address of the person who shared it with you.

Comment

[github-actions]

Hi @andfinally, @raja651,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

[andfinally]

This is a brilliant improvement, thanks! 👌

I wonder if there's anything we can do about the tooltip width? With my (admittedly unnaturally long) test email address, I get an ugly scrollbar.

Also, I just noticed the behaviour of this and the tooltip on the Expired badge is a bit annoying. It won't go away till you click the badge again. This is not very intuitive, IMHO. I notice we use the same component for the kebab menu, but I think we add an outside click handler when we show it, so you can close it by clicking anywhere outside it.

[Dan-Q]

I wonder if there's anything we can do about the tooltip width? With my (admittedly unnaturally long) test email address, I get an ugly scrollbar.

We can't change the width of the tooltip to anything sensible without breaking all kinds of other bits of design. How about we just make it so that exceptionally-long email addresses are allowed to wrap. It's the least-bad option, and only requires a single word-wrap: anywhere; CSS directive:

Screen.Recording.2024-04-16.at.19.21.30.mov

[Dan-Q]

Also, I just noticed the behaviour of this and the tooltip on the Expired badge is a bit annoying. It won't go away till you click the badge again. This is not very intuitive, IMHO. I notice we use the same component for the kebab menu, but I think we add an outside click handler when we show it, so you can close it by clicking anywhere outside it.

Suggest this is treated as a separate issue, as (a) it applies to all the <StatusPopover>s everwhere, and (b) should probably be fixed centrally for them all, which is kinda out-of-scope for this issue.

[Dan-Q]

I wonder if there's anything we can do about the tooltip width? With my (admittedly unnaturally long) test email address, I get an ugly scrollbar.

We can't change the width of the tooltip to anything sensible without breaking all kinds of other bits of design. How about we just make it so that exceptionally-long email addresses are allowed to wrap. It's the least-bad option, and only requires a single word-wrap: anywhere; CSS directive:

Done in e636709.

[andfinally]

Thanks, looks good! I get your points about the tooltip width and behaviour.

Just one possible quirk I can see – when I click to reveal the tooltip, the "Learn more" link has its accessibility outline.

[Dan-Q]

Just one possible quirk I can see – when I click to reveal the tooltip, the "Learn more" link has its accessibility outline.

Yeah, I noticed that: I assumed that it was a deliberate (a11y?) choice for this component to focus the contents of the tooltip when opened. I'll double-check!

[andfinally]

Hi @Dan-Q, just a nudge about this PR! I'm getting a webpack build error locally – I tried to rebase on trunk, but ran into several conflicts.

[Dan-Q]

Hi @Dan-Q, just a nudge about this PR! I'm getting a webpack build error locally – I tried to rebase on trunk, but ran into several conflicts.

I just merged changes from trunk without any conflicts, and I'm not getting build errors. What errors are you getting @andfinally?

[Dan-Q]

Ah! Found the build error. Looks like the underlying TypeScript type doesn't anticipate an actually-valid CSS declaration. Lemme fix that!

[github-actions]

Test using WordPress Playground

The changes in this pull request can be previewed and tested using a WordPress Playground instance.
WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Test this pull request with WordPress Playground.

Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit.

[Dan-Q]

Okay @andfinally! Try that now!

[andfinally]

Thanks, works as expected!

I'm still getting the horrible outline I mentioned in #46229 (review). It's coming from a WordPress rule on all links in common.css.

I'd suggest we override it.

Index: plugins/woocommerce-admin/client/marketplace/components/my-subscriptions/my-subscriptions.scss
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/plugins/woocommerce-admin/client/marketplace/components/my-subscriptions/my-subscriptions.scss b/plugins/woocommerce-admin/client/marketplace/components/my-subscriptions/my-subscriptions.scss
--- a/plugins/woocommerce-admin/client/marketplace/components/my-subscriptions/my-subscriptions.scss	(revision d627bcdcd532e1f0a34469b9f03f5a6411832c6c)
+++ b/plugins/woocommerce-admin/client/marketplace/components/my-subscriptions/my-subscriptions.scss	(date 1715767787120)
@@ -198,6 +198,10 @@
 		color: $gray-900;
 		a {
 			text-decoration: none;
+
+			&:focus {
+				box-shadow: none;
+			}
 		}
 	}
 }

I'd even argue StatusPopover needs a pointer cursor, as it's interactive.

[Dan-Q]

I'm still getting the horrible outline I mentioned in #46229 (review). It's coming from a WordPress rule on all links in common.css.

I'd suggest we override it.

If we override it in the way you suggest, it's bad for a11y. Instead, how about we just don't focus-within the popover when it appears? [2a36975] Then the link still gets a focus state when it's focussed, but not just when the popup appears.

Screen.Recording.2024-05-17.at.12.36.26.mov

[andfinally]

I'm confused why we're highlighting the link if the focus is on its parent. And half of me thinks we focus on the tooltip by deafult for accessibility reasons – for example to make it easier to tab through to any link inside it. But this looks like an easy change that resolves the focus issue, thanks!

[Dan-Q]

I'm confused why we're highlighting the link if the focus is on its parent.

It looks like the default behaviour for a <Popover> is to focus on the first focussable target within it, to make keyboard navigation easier if e.g. you're popping up a toolbar.