Update share limits bar design by manzoorwanijk · Pull Request #34392 · Automattic/jetpack

manzoorwanijk · 2023-11-30T13:40:22Z

Proposed changes:

Update the share limits bar design to match the Figma design - 25F3UyoNr6TUSWBplnpRyM-fi-4007%3A5874

Other information:

Have you written new tests for your changes, if applicable?
Have you checked the E2E test CI results, and verified that your changes do not break them?
Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Create/edit a post on a site without Jetpack Social plan
Confirm that the limits bar works as expected on the sidebar of the editor
Goto Jetpack Social admin page
Confirm that the bar works as expected

Different scenarios

Used: 20, Scheduled: 0
Used: 25, Scheduled: 0
Used: 25, Scheduled: 3
Used: 25, Scheduled: 5
Used: 25, Scheduled: 10
Used: 30, Scheduled: 0
Jetpack Social admin page

github-actions · 2023-11-30T13:41:35Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team Review" label and ask someone from your team review the code. Once reviewed, it can then be merged.
If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.

Social plugin:

Next scheduled release: January 2, 2024.
Scheduled code freeze: December 25, 2023.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

github-actions · 2023-11-30T13:42:14Z

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

To test on WoA, go to the Plugins menu on a WordPress.com Simple site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin, and enable the update/share-limits-bar-design branch.

To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack update/share-limits-bar-design

Interested in more tips and information?

In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
Read more about our development workflow here: PCYsg-eg0-p2
Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

projects/js-packages/publicize-components/src/components/share-limits-bar/index.tsx

pablinos

There's still a fair amount to do here:

Take into account all the other logic around whether a connection is enabled/disabled. If a connection is broken, invalid (like with media requirements for Instagram), etc. Then we will still count it as 'enabled' for the share counts. The logic for this probably needs to move into the useSocialMediaConnections hook so we can set the disabled property on each connection correctly. This might well be broken on trunk too, so we could also do it as a follow-up
We need to fix the logic around a post date being set inside or outside of the 30 day window we look at for scheduled shares. I've added some comments on what we could do.
Add the line "This post will be shared to x connection(s)". I was thinking instead of saying "This post won't be shared to any connections" when there aren't any enabled, we could say something like "Enable a connection to share this post".
Fix the styling of the bar in the Social admin page
Fix the tests!

projects/js-packages/publicize-components/src/social-store/resolvers.js

projects/js-packages/publicize-components/src/social-store/selectors/shares-data.ts

projects/plugins/social/src/js/components/header/index.js

projects/js-packages/components/components/record-meter-bar/index.tsx

projects/js-packages/publicize-components/src/components/form/share-count-notice.tsx

projects/js-packages/publicize-components/src/components/share-limits-bar/index.tsx

projects/js-packages/publicize-components/src/hooks/use-scheduled-post/index.js

projects/js-packages/publicize-components/src/hooks/use-share-limits/index.ts

pablinos · 2023-12-04T00:31:28Z

We also need to update the notice styling so the upgrade link is a button, ande we adde the Learn more link.

I think we should always have an upgrade link whether there is a warning or not.

manzoorwanijk · 2023-12-07T07:10:35Z

I know this is a variation on the designs, but because we're not using the current post in the meter anymore, I wondered if it should turn red sooner. We change it to yellow when there are 20% of shares remaining, maybe it could go red with 5-10% remaining? This is just a thought though, feel free to stick to the designs!

For a limit of 30 shares, 80% sounds like a good choice because they will have only 6 shares left. If we make it 90%, they will have only 3 shares left.

manzoorwanijk · 2023-12-07T08:13:11Z

I know we said to keep it not reactivity but what do you think about adding the 'This post will be shared to x connections' line? We should be able to filter for all the invalid ones too, so we don't have the problem with Instagram looking like it's enabled.

Done ✅

projects/js-packages/publicize-components/src/components/form/styles.module.scss

manzoorwanijk · 2023-12-07T08:42:05Z

On the admin page it looks like there is a lot of space between the bottom of the legend an the upgrade nudge. I wonder if we should remove the padding and rely on the upgrade nudge top margin?

I have removed the padding on the admin page.

gmjuhasz

Let's merge it into the main branch and give it a final test

pablinos · 2023-12-07T09:25:24Z

I know we said to keep it not reactivity but what do you think about adding the 'This post will be shared to x connections' line? We should be able to filter for all the invalid ones too, so we don't have the problem with Instagram looking like it's enabled.

Done ✅

Thanks! Should we have something like "Enable a connection to share this post when it is published" or something along those lines, when there are none selected? That might be repeating what gets displayed in the text above the connections though.

pablinos · 2023-12-07T09:26:25Z

I know this is a variation on the designs, but because we're not using the current post in the meter anymore, I wondered if it should turn red sooner. We change it to yellow when there are 20% of shares remaining, maybe it could go red with 5-10% remaining? This is just a thought though, feel free to stick to the designs!

For a limit of 30 shares, 80% sounds like a good choice because they will have only 6 shares left. If we make it 90%, they will have only 3 shares left.

Yeah I was thinking that it turns red at 100% currently. Maybe as it's not reactive now that would be better to happen at 90%?

pablinos · 2023-12-07T09:28:03Z

I think it looks good as a warning always. The bar being green is enough to show that they are under the safe limits.

As mentioned in Slack, I think having it as a warning always will make people blind to it. I think we should only 'warn' them when there is something to warn them about. I was worried about jank with the notice moving from below to above the connections, but now it's not reactive, that shouldn't be a problem.

manzoorwanijk · 2023-12-07T09:28:12Z

Should we have something like "Enable a connection to share this post when it is published" or something along those lines, when there are none selected? That might be repeating what gets displayed in the text above the connections though.

It is visible for the posts that are already published. So, "...when it is published" won't make sense.

pablinos · 2023-12-07T09:28:54Z

Looking at the designs it seems that although the grey in the legend looks darker, the bar is actually a bit lighter, which I think is better a colour because otherwise the light green for the 'scheduled' count, gets a little lost.

If you inspect that in the dev tools, both are the same - --jp-gray-off

Sure, but in the designs, the bar at least is #F6F7F7 which is much lighter

manzoorwanijk · 2023-12-07T09:30:42Z

I was thinking that it turns red at 100% currently. Maybe as it's not reactive now that would be better to happen at 90%?

That will further complicate the logic. May be we can do that in a follow up PR.

manzoorwanijk · 2023-12-07T09:33:40Z

in the designs, the bar at least is #F6F7F7 which is much lighter

That is --jp-gray-0 which gets lost in the admin page

manzoorwanijk · 2023-12-07T09:37:41Z

As mentioned in Slack, I think having it as a warning always will make people blind to it. I think we should only 'warn' them when there is something to warn them about. I was worried about jank with the notice moving from below to above the connections, but now it's not reactive, that shouldn't be a problem.

I have updated it to look like this

…f notice" This reverts commit b0b1909.

manzoorwanijk · 2023-12-07T10:31:06Z

Here is the updated look of the title and the notice

* Make share data selectors independent of the connections * Enhance RecordMeterBar component * Enhance ShareLimitsBar component * Pass connections data to useShareLimits * Update editor form components * Update social admin page header * Add support for providing text variant * Add enabledConnections to used/scheduled number * Fix enabledConnections being NaN * Update the notice and meter to match the new designs/logic. * Update the admin page header component * Fix tests * Revert changes to RecordBarMeter * Revert an unnecessary change to RecordMeterBar * Add changelog * Create update-share-limits-bar-design * Fix "msgid argument is not a string literal" build error 100th time in Jetpack * Fix "Optimization seems to have broken the following translation strings" - "left" * Revert "Fix "Optimization seems to have broken the following translation strings" - "left"" This reverts commit d1d7cda. * Use "remaining" instead of "left" for the sake of passing the build * Fix "Optimization seems to have broken the following translation strings - "left"" build error * Simplify share limits bar logic * Clean up unused logic * Restore outOfConnections logic * Fix unit tests * Clean up types.ts * Update notice CTA design * Use "or" instead of "and" * Fix the bar title and caption * Fix "Optimization seems to have broken the following translation strings" * Fix typo * Make it a minor change instead of a patch * Add enabled connections text * Remove padding for the bar on Social admin page * Fix up versions * When under the limits, show an inline upgrade nudge instead of notice * Revert "When under the limits, show an inline upgrade nudge instead of notice" This reverts commit b0b1909. * Fix the order of the title and notice * Clean things up a bit --------- Co-authored-by: Gergely Juhasz <gergely.juhasz@automattic.com> Co-authored-by: Paul Bunkham <paul@dobit.co.uk>

* Enhance RecordBarMeter component with `className` * Add changelog * Jetpack Social | Warn the user that their scheduled posts won't get shared if they will get over limits. (#34183) * Enhance RecordBarMeter component with `className` * Add changelog * Add store types * Update store selectors and actions * Update consumer hook to use new selectors * Create useShareLimits hook * Move `ShareCounter` to publicize-components and name it `ShareLimitsBar` * Create separate notice components * Fix the post editor notices * Update Jetpack Social admin page to use the new meter bar * Remove unused code left after #34111 * Add changelog * Fix share count * Update index.js * Include active connections count for notices * Fix i18n build error * Enhance RecordBarMeter component with `className` * Add changelog * Extract constants and improve types * Extract messages to utility function * Enhance RecordBarMeter component with `className` * Add changelog * Fix up versions * Revert "Remove unused code left after #34111" This reverts commit ca45999. * Use @wordpress/element instead of react * Fix messaging * Let TS infer type * Combine the share limits logic into a single useSelect call * Fix the limits shown even after a paid plan * Use boolean instead of number * Add unit tests for sharing data store (#34206) * Enhance RecordBarMeter component with `className` * Add changelog * Add store types * Update store selectors and actions * Update consumer hook to use new selectors * Create useShareLimits hook * Move `ShareCounter` to publicize-components and name it `ShareLimitsBar` * Create separate notice components * Fix the post editor notices * Update Jetpack Social admin page to use the new meter bar * Remove unused code left after #34111 * Add changelog * Fix share count * Update index.js * Include active connections count for notices * Fix i18n build error * Enhance RecordBarMeter component with `className` * Add changelog * Extract constants and improve types * Add types/jest * Add tests for sharesData selectors * Add tests for useShareLimits hook * rename to *.test.js * extract utility * Create add-tests-for-sharing-data * Update shares-data.test.js * Extract messages to utility function * Use the messages from utility function * Enhance RecordBarMeter component with `className` * Add changelog * Fix up versions * Revert "Remove unused code left after #34111" This reverts commit ca45999. * Use @wordpress/element instead of react * Fix messaging * Let TS infer type * Combine the share limits logic into a single useSelect call * Fix the limits shown even after a paid plan * Enhance RecordBarMeter component with `className` * Add changelog * Enhance RecordBarMeter component with `className` * Add changelog * Jetpack Social | Warn the user that their scheduled posts won't get shared if they will get over limits. (#34183) * Enhance RecordBarMeter component with `className` * Add changelog * Add store types * Update store selectors and actions * Update consumer hook to use new selectors * Create useShareLimits hook * Move `ShareCounter` to publicize-components and name it `ShareLimitsBar` * Create separate notice components * Fix the post editor notices * Update Jetpack Social admin page to use the new meter bar * Remove unused code left after #34111 * Add changelog * Fix share count * Update index.js * Include active connections count for notices * Fix i18n build error * Enhance RecordBarMeter component with `className` * Add changelog * Extract constants and improve types * Extract messages to utility function * Enhance RecordBarMeter component with `className` * Add changelog * Fix up versions * Revert "Remove unused code left after #34111" This reverts commit ca45999. * Use @wordpress/element instead of react * Fix messaging * Let TS infer type * Combine the share limits logic into a single useSelect call * Fix the limits shown even after a paid plan * Update tests * Fix up versions * Jetpack Social| Fix scheduled post double count (#34323) * Update store selectors and resolvers to save initial connections * Update useShareLimits hook to subtract scheduled post if editing one * Update ShareCountInfo to subtract the scheduled post from count * Add changelog * Fix versions * changelog!!!! * Update share limits bar design (#34392) * Make share data selectors independent of the connections * Enhance RecordMeterBar component * Enhance ShareLimitsBar component * Pass connections data to useShareLimits * Update editor form components * Update social admin page header * Add support for providing text variant * Add enabledConnections to used/scheduled number * Fix enabledConnections being NaN * Update the notice and meter to match the new designs/logic. * Update the admin page header component * Fix tests * Revert changes to RecordBarMeter * Revert an unnecessary change to RecordMeterBar * Add changelog * Create update-share-limits-bar-design * Fix "msgid argument is not a string literal" build error 100th time in Jetpack * Fix "Optimization seems to have broken the following translation strings" - "left" * Revert "Fix "Optimization seems to have broken the following translation strings" - "left"" This reverts commit d1d7cda. * Use "remaining" instead of "left" for the sake of passing the build * Fix "Optimization seems to have broken the following translation strings - "left"" build error * Simplify share limits bar logic * Clean up unused logic * Restore outOfConnections logic * Fix unit tests * Clean up types.ts * Update notice CTA design * Use "or" instead of "and" * Fix the bar title and caption * Fix "Optimization seems to have broken the following translation strings" * Fix typo * Make it a minor change instead of a patch * Add enabled connections text * Remove padding for the bar on Social admin page * Fix up versions * When under the limits, show an inline upgrade nudge instead of notice * Revert "When under the limits, show an inline upgrade nudge instead of notice" This reverts commit b0b1909. * Fix the order of the title and notice * Clean things up a bit --------- Co-authored-by: Gergely Juhasz <gergely.juhasz@automattic.com> Co-authored-by: Paul Bunkham <paul@dobit.co.uk> * Fix up versions and lockfile * Fix the bar width * Fix TS build * Restore dependencies messed up while resolving conflicts. * Restore the record meter bar styles * Make the bar go red after 90% usage * Use "connection" instead of "network" * Hide connections text when publicize is OFF * Fix up versions --------- Co-authored-by: Gergely Juhasz <gergely.juhasz@automattic.com> Co-authored-by: Paul Bunkham <paul@dobit.co.uk>

manzoorwanijk added 6 commits November 30, 2023 19:07

Make share data selectors independent of the connections

33c80aa

Enhance RecordMeterBar component

04a623f

Enhance ShareLimitsBar component

e814e1f

Pass connections data to useShareLimits

efa10b2

Update editor form components

8e7cc0a

Update social admin page header

9d82433

manzoorwanijk self-assigned this Nov 30, 2023

github-actions bot added [JS Package] Components [JS Package] Publicize Components [Package] Publicize [Plugin] Social Issues about the Jetpack Social plugin [Status] In Progress [Tests] Includes Tests RNA labels Nov 30, 2023

pablinos changed the base branch from trunk to feature/jetpack-social-scheduled-posts-ui November 30, 2023 14:35

gmjuhasz added 3 commits December 1, 2023 13:12

Add support for providing text variant

1116ef3

Add enabledConnections to used/scheduled number

d080a69

Fix enabledConnections being NaN

6ef25b6

manzoorwanijk commented Dec 1, 2023

View reviewed changes

projects/js-packages/publicize-components/src/components/share-limits-bar/index.tsx Outdated Show resolved Hide resolved

Update the notice and meter to match the new designs/logic.

69e4cff

pablinos reviewed Dec 4, 2023

View reviewed changes

pablinos and others added 6 commits December 4, 2023 00:42

Update the admin page header component

2cdcfe5

Fix tests

e3f5101

Revert changes to RecordBarMeter

8aa5399

Revert an unnecessary change to RecordMeterBar

1023f74

Add changelog

70a4cc8

Create update-share-limits-bar-design

fbd37a5

Add enabled connections text

3149084

manzoorwanijk commented Dec 7, 2023

View reviewed changes

projects/js-packages/publicize-components/src/components/form/styles.module.scss Show resolved Hide resolved

Remove padding for the bar on Social admin page

7438daa

manzoorwanijk requested a review from a team December 7, 2023 08:42

Fix up versions

02f7fe1

gmjuhasz approved these changes Dec 7, 2023

View reviewed changes

When under the limits, show an inline upgrade nudge instead of notice

b0b1909

manzoorwanijk added 2 commits December 7, 2023 15:10

Revert "When under the limits, show an inline upgrade nudge instead o…

98ed700

…f notice" This reverts commit b0b1909.

Fix the order of the title and notice

c2651cf

Clean things up a bit

7743385

manzoorwanijk merged this pull request into feature/jetpack-social-scheduled-posts-ui Dec 7, 2023

manzoorwanijk deleted the update/share-limits-bar-design branch December 7, 2023 10:39

github-actions bot removed the [Status] In Progress label Dec 7, 2023

Conversation

manzoorwanijk commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes:

Other information:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Uh oh!

github-actions bot commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

pablinos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pablinos commented Dec 4, 2023

Uh oh!

manzoorwanijk commented Dec 7, 2023

Uh oh!

manzoorwanijk commented Dec 7, 2023

Uh oh!

Uh oh!

manzoorwanijk commented Dec 7, 2023

Uh oh!

gmjuhasz left a comment

Choose a reason for hiding this comment

Uh oh!

pablinos commented Dec 7, 2023

Uh oh!

pablinos commented Dec 7, 2023

Uh oh!

pablinos commented Dec 7, 2023

Uh oh!

manzoorwanijk commented Dec 7, 2023

Uh oh!

pablinos commented Dec 7, 2023

Uh oh!

manzoorwanijk commented Dec 7, 2023

Uh oh!

manzoorwanijk commented Dec 7, 2023

Uh oh!

manzoorwanijk commented Dec 7, 2023

Uh oh!

manzoorwanijk commented Dec 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

manzoorwanijk commented Nov 30, 2023 •

edited

Loading

github-actions bot commented Nov 30, 2023 •

edited

Loading

github-actions bot commented Nov 30, 2023 •

edited

Loading