Calypsoify: Copy the module code to the Calypsoify package #37339

coder-karen · 2024-05-10T08:14:14Z

Resolves https://github.com/Automattic/vulcan/issues/319

Proposed changes:

Copy the Calypsoify code from the Jetpack module into the newly introduced Calypsoify PHP package.
Minor adjustments - adding namespace, removing unnecessary mods.js file, moving JS files to JS folder and CSS to a CSS folder, plus readme tweaks.
Additionally, updated how the Jetpack version constant is retrieved and re-added the phan baseline file due to the remaining phan test results (the rest appear to be false positives or issues that previously existed, though I'll be able to clarify any issues when actually hooking everything up).

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

https://github.com/Automattic/vulcan/issues/319
pfwV0U-8E-p2

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

No.

Testing instructions:

This package is not connected to anything yet so a generic review that everything appears to be in place should be enough. More will likely be needed when the package is required, but that will come later.
That said, the build can be tested by checking out this PR and running jetpack build packages/calypsoify --production. You should see a 'build' directory created with 3 files - the minified JS, CSS and a RTL CSS file.

github-actions · 2024-05-10T08:15:09Z

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.

… the package version

…kage version

…aught

coder-karen · 2024-05-10T10:12:43Z

projects/packages/calypsoify/.phan/baseline.php

+    'file_suppressions' => [
+        'src/class-jetpack-calypsoify.php' => ['PhanTypeMismatchArgumentProbablyReal', 'PhanTypeMismatchPropertyDefault', 'PhanUndeclaredClassMethod'],


I re-created the baseline file, and ran jetpack phan packages/calypsoify --update-baseline which automatically added these suppressions. I had previously fixed an issue with the Jetpack version constant declaration which was flagged by phan test results - the remaining issues appear to be false positives or issues that previously existed, though I'll be able to clarify any issues when actually hooking everything up in the follow-up PR.

Edit to add the file has been updated now, so the only ignores are two pre-existing issues from what I can see, and I'll be able to test out possible fixes if needed when hooking everything up:

Error: TypeError PhanTypeMismatchPropertyDefault Default value for object $instance can't be false of type false based on phpdoc types projects/packages/calypsoify/src/class-jetpack-calypsoify.php:70 Error: TypeError PhanTypeMismatchArgumentProbablyReal Argument 3 ($deps) is false of type false but \wp_enqueue_style() takes array|string[] (no real type) defined at /home/runner/work/jetpack/jetpack/vendor/php-stubs/wordpress-stubs/wordpress-stubs.php:114134 (the inferred real argument type has nothing in common with the parameter's phpdoc type) projects/packages/calypsoify/src/class-jetpack-calypsoify.php:71

fgiannar · 2024-05-10T13:47:52Z

projects/packages/calypsoify/src/class-jetpack-calypsoify.php

+	 */
+	public function enqueue_for_gutenberg() {
+		$site_suffix = ( new Status() )->get_site_suffix();
+		wp_enqueue_style( 'calypsoify_wpadminmods_css', plugin_dir_url( __FILE__ ) . 'style-gutenberg.min.css', false, self::PACKAGE_VERSION );


Since we changed the file structure with css files currently living under the css folder we'll need to fix the path here as well.
I guess the style-gutenberg.min.css will be created on build but added to the .gitignore too?

Good point, thanks!

fgiannar · 2024-05-10T13:48:11Z

projects/packages/calypsoify/src/class-jetpack-calypsoify.php

+		wp_style_add_data( 'calypsoify_wpadminmods_css', 'rtl', 'replace' );
+		wp_style_add_data( 'calypsoify_wpadminmods_css', 'suffix', '.min' );
+
+		wp_enqueue_script( 'calypsoify_wpadminmods_js', plugin_dir_url( __FILE__ ) . 'mods-gutenberg.js', array( 'jquery' ), self::PACKAGE_VERSION, false );


Since we changed the file structure with js files currently living under the js folder we'll need to fix the path here as well.

…paths

github-actions · 2024-05-10T15:45:05Z

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 add/calypsoify-package-code branch.
- For jetpack-mu-wpcom changes, also add define( 'JETPACK_MU_WPCOM_LOAD_VIA_BETA_PLUGIN', true ); to your wp-config.php file.

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

bin/jetpack-downloader test jetpack add/calypsoify-package-code

bin/jetpack-downloader test jetpack-mu-wpcom-plugin add/calypsoify-package-code

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

coder-karen · 2024-05-10T15:54:21Z

projects/packages/calypsoify/src/class-jetpack-calypsoify.php

+		wp_enqueue_style( 'calypsoify_wpadminmods_css', plugin_dir_url( __FILE__ ) . 'build/style-gutenberg.css', false, self::PACKAGE_VERSION );
+		wp_style_add_data( 'calypsoify_wpadminmods_css', 'rtl', 'replace' );
+		wp_style_add_data( 'calypsoify_wpadminmods_css', 'suffix', '.min' );
+
+		wp_enqueue_script( 'calypsoify_wpadminmods_js', plugin_dir_url( __FILE__ ) . 'build/mods-gutenberg.js', array( 'jquery' ), self::PACKAGE_VERSION, false );


After changing the paths, I will need to double check this is all working correctly once hooking everything up in the next PR, but this should be ok to leave as is for this PR.
For example the .min file is actually the current .css file, so I will need to either remove this line or change the build output depending on whether it will work to have no defined min file, but will investigate more later.

Yeah I'm also not sure what is the best way to handle rtl.
Based on my understanding so far, webpack by default will create an rtl file for eg style.css named style.rtl.css however wordpress expects style-rtl.css when setting the rtl property.

Ideally the JS files should be built with our normal monorepo webpack config and then we should use our jetpack-assets package to enqueue things with Assets::register_script(). That handles a lot of the various bits for us.

I have no idea whether making that happen here would be a lot of work though.

…ge-code

fgiannar

Let's 🚢 this to unblock you and we can fully test if assets are properly enqueued/rtl respected etc in a follow up PR :)

coder-karen added 2 commits May 10, 2024 09:06

Copying and moving the Calypsoify code to the Calypsoify package

1002a21

changelog

99edeb2

coder-karen added the [Status] In Progress label May 10, 2024

coder-karen self-assigned this May 10, 2024

github-actions bot added [Feature] Calypsoify [Package] Calypsoify Docs labels May 10, 2024

coder-karen added 3 commits May 10, 2024 10:26

Modifying how the Jetpack conversion constant is received, and adding…

056e97d

… the package version

Update composer.json to include the correct file path to find the pac…

8955ba1

…kage version

Re-add the phan baseline file to make sure phan false positives are c…

fb28039

…aught

coder-karen marked this pull request as ready for review May 10, 2024 09:58

coder-karen commented May 10, 2024

View reviewed changes

coder-karen added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels May 10, 2024

coder-karen requested a review from a team May 10, 2024 10:30

coder-karen added [Status] In Progress and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels May 10, 2024

coder-karen removed the request for review from a team May 10, 2024 12:07

Updating phan as well as the package version const and package reqs

d5be7c7

fgiannar reviewed May 10, 2024

View reviewed changes

coder-karen added 2 commits May 10, 2024 16:40

Adding a webpack build to build CSS and JS files, and update related …

d24a308

…paths

Update the lock file

b9c39b5

coder-karen commented May 10, 2024

View reviewed changes

coder-karen added 2 commits May 13, 2024 09:38

Tweak package.json build steps

0de549a

Merge remote-tracking branch 'origin/trunk' into add/calypsoify-packa…

db53908

…ge-code

coder-karen added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels May 13, 2024

fgiannar approved these changes May 13, 2024

View reviewed changes

fgiannar added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels May 13, 2024

coder-karen merged commit 51c9466 into trunk May 13, 2024
74 checks passed

coder-karen deleted the add/calypsoify-package-code branch May 13, 2024 12:41

github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label May 13, 2024

fgiannar mentioned this pull request May 13, 2024

Masterbar: Copy module code to package #37342

Merged

3 tasks

coder-karen mentioned this pull request Jun 13, 2024

Custom CSS: copy module to jetpack-mu-wpcom package #37794

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Calypsoify: Copy the module code to the Calypsoify package #37339

Calypsoify: Copy the module code to the Calypsoify package #37339

coder-karen commented May 10, 2024 •

edited

Loading

github-actions bot commented May 10, 2024

coder-karen May 10, 2024

coder-karen May 10, 2024

fgiannar May 10, 2024

coder-karen May 10, 2024

fgiannar May 10, 2024

github-actions bot commented May 10, 2024 •

edited

Loading

coder-karen May 10, 2024 •

edited

Loading

fgiannar May 13, 2024

anomiex May 13, 2024

fgiannar left a comment

		'file_suppressions' => [
		'src/class-jetpack-calypsoify.php' => ['PhanTypeMismatchArgumentProbablyReal', 'PhanTypeMismatchPropertyDefault', 'PhanUndeclaredClassMethod'],

Calypsoify: Copy the module code to the Calypsoify package #37339

Calypsoify: Copy the module code to the Calypsoify package #37339

Conversation

coder-karen commented May 10, 2024 • edited Loading

Proposed changes:

Other information:

Jetpack product discussion

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

Testing instructions:

github-actions bot commented May 10, 2024

coder-karen May 10, 2024

Choose a reason for hiding this comment

coder-karen May 10, 2024

Choose a reason for hiding this comment

fgiannar May 10, 2024

Choose a reason for hiding this comment

coder-karen May 10, 2024

Choose a reason for hiding this comment

fgiannar May 10, 2024

Choose a reason for hiding this comment

github-actions bot commented May 10, 2024 • edited Loading

coder-karen May 10, 2024 • edited Loading

Choose a reason for hiding this comment

fgiannar May 13, 2024

Choose a reason for hiding this comment

anomiex May 13, 2024

Choose a reason for hiding this comment

fgiannar left a comment

Choose a reason for hiding this comment

coder-karen commented May 10, 2024 •

edited

Loading

github-actions bot commented May 10, 2024 •

edited

Loading

coder-karen May 10, 2024 •

edited

Loading