Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jetpack Social| Fix scheduled posts UI #34182

Merged
merged 17 commits into from
Dec 13, 2023

Conversation

manzoorwanijk
Copy link
Member

@manzoorwanijk manzoorwanijk commented Nov 17, 2023

This is the base branch for fixing scheduled posts UI for Jetpack Social

Proposed changes:

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:

  • This is a base branch
  • Please test the individual brances

@manzoorwanijk manzoorwanijk added the DO NOT MERGE don't merge it! label Nov 17, 2023
@manzoorwanijk manzoorwanijk self-assigned this Nov 17, 2023
Copy link
Contributor

github-actions bot commented Nov 17, 2023

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 feature/jetpack-social-scheduled-posts-ui branch.

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

    bin/jetpack-downloader test jetpack feature/jetpack-social-scheduled-posts-ui
    

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

Copy link
Contributor

github-actions bot commented Nov 17, 2023

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.

@manzoorwanijk manzoorwanijk force-pushed the feature/jetpack-social-scheduled-posts-ui branch from ea04d2f to f46952a Compare November 20, 2023 05:45
@manzoorwanijk manzoorwanijk force-pushed the feature/jetpack-social-scheduled-posts-ui branch 3 times, most recently from 51c428c to d55655d Compare November 22, 2023 06:51
@manzoorwanijk manzoorwanijk force-pushed the feature/jetpack-social-scheduled-posts-ui branch 4 times, most recently from f5ba835 to 073a3df Compare November 28, 2023 04:42
@manzoorwanijk manzoorwanijk force-pushed the feature/jetpack-social-scheduled-posts-ui branch 2 times, most recently from dfb3268 to 078d13c Compare November 30, 2023 06:38
@manzoorwanijk manzoorwanijk force-pushed the feature/jetpack-social-scheduled-posts-ui branch from 88db9dc to 4ea62b0 Compare December 7, 2023 10:42
@manzoorwanijk manzoorwanijk requested a review from a team December 7, 2023 10:47
@manzoorwanijk manzoorwanijk marked this pull request as ready for review December 7, 2023 10:47
Copy link
Contributor

@gmjuhasz gmjuhasz left a comment

Choose a reason for hiding this comment

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

I found a minor issue. If you have 0 schedule shares, the bar looks wrong, it's not filling the space out. If I schedule at least 1 post it looks good.

CleanShot 2023-12-07 at 12 51 16 png

@manzoorwanijk
Copy link
Member Author

I found a minor issue. If you have 0 schedule shares, the bar looks wrong, it's not filling the space out. If I schedule at least 1 post it looks good.

Thank you for catching that. I have pushed a fix

Screenshot 2023-12-07 at 5 55 17 PM

gmjuhasz
gmjuhasz previously approved these changes Dec 7, 2023
Copy link
Contributor

@gmjuhasz gmjuhasz left a comment

Choose a reason for hiding this comment

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

Thanks for fixing that. I tested the scenarios and all seemed to work for me 👍

@manzoorwanijk manzoorwanijk force-pushed the feature/jetpack-social-scheduled-posts-ui branch from d62caa3 to f8a15bb Compare December 7, 2023 12:59
Copy link
Contributor

@pablinos pablinos left a comment

Choose a reason for hiding this comment

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

I've mainly just scanned through the code again, but a lot of it has been reviewed as part of other PRs.

We should check a few scenarios, mainly with and without a plan on both the Jetpack and Social plugins. We should also test this on WPCOM. We've been seeing some conflicts with different versions of plugins active on sites too, so we should probably check this in that scenario too.

@manzoorwanijk manzoorwanijk force-pushed the feature/jetpack-social-scheduled-posts-ui branch from f914126 to f8c2557 Compare December 11, 2023 06:04
manzoorwanijk and others added 16 commits December 12, 2023 15:08
…hared 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
* 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
* 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!!!!
* 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 <[email protected]>
Co-authored-by: Paul Bunkham <[email protected]>
@manzoorwanijk manzoorwanijk force-pushed the feature/jetpack-social-scheduled-posts-ui branch from 0621d9a to 8e120f3 Compare December 12, 2023 09:39
@manzoorwanijk manzoorwanijk enabled auto-merge (squash) December 12, 2023 10:11
Copy link
Contributor

@spsiddarthan spsiddarthan left a comment

Choose a reason for hiding this comment

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

Tests well!

@manzoorwanijk manzoorwanijk merged commit 5858f1e into trunk Dec 13, 2023
66 checks passed
@manzoorwanijk manzoorwanijk deleted the feature/jetpack-social-scheduled-posts-ui branch December 13, 2023 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants