Skip to content
This repository has been archived by the owner on Oct 19, 2022. It is now read-only.

Add a notice when requirements aren’t met #75

Merged
merged 4 commits into from
Nov 2, 2018

Conversation

pyronaur
Copy link
Member

@pyronaur pyronaur commented Nov 1, 2018

This adds a notice in WordPress < 5.0 if Gutenberg is also below version 3.5.0 because Ramp doesn’t work with Gutenberg plugin that old anymore.

This is what the notice looks like:

screen shot 2018-11-01 at 5 47 34 pm

The notice is a bit hacky in that it changes based on date. Anything after November 20, 2018 will append "Or upgrade to WordPress 5.0" in the notice based on a timestamp. An available WordPress update could be checked with wp_version_check() but I think it just adds too much overhead for such a small edge case that I opted for the simplest solution here, even though it may not be perfectly accurate. Let me know if you think the notice should rely on wp_version_check() instead or even be worded differently. There's only 1 type of notice now when the PR is updated 🎉

This PR also extracts code from gutenberg_ramp_require_gutenberg() into a new function: gutenberg_ramp_validated_gutenberg_load_path - this is because apply_filters() is used and the Gutenberg path can be modified, so instead of manually applying filters twice - I've moved the filter to a dedicated function.

This adds a notice in WordPress < 5.0 if Gutenberg is also below version 3.5.0 because Ramp doesn’t work with Gutenberg plugin that old anymore.
Copy link
Member

@mjangda mjangda left a comment

Choose a reason for hiding this comment

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

Left some minor notes.

gutenberg-ramp.php Outdated Show resolved Hide resolved
gutenberg-ramp.php Outdated Show resolved Hide resolved
inc/admin/class-gutenberg-ramp-compatibility-check.php Outdated Show resolved Hide resolved
inc/admin/class-gutenberg-ramp-compatibility-check.php Outdated Show resolved Hide resolved
@@ -148,6 +162,11 @@ function gutenberg_ramp_require_gutenberg() {
*/
gutenberg_ramp_require_gutenberg();

if ( Gutenberg_Ramp_Compatibility_Check::should_check_compatibility() ) {
$ramp_compatibility = new Gutenberg_Ramp_Compatibility_Check();
Copy link
Member

Choose a reason for hiding this comment

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

Should we skip loading the rest of Ramp if the versions don't match?

Copy link
Member Author

@pyronaur pyronaur Nov 2, 2018

Choose a reason for hiding this comment

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

I don't think it's necessary. If Ramp isn't compatible - it doesn't do anything apart from attaching itself to 2 filters that will never be called, but it still does allow you to interact with ramp helper the you would normally.

I'm hesitant to change this because this is an edge case we're talking about. Refactoring the plugin to activate on a condition just because of an edge seems like a bit of overkill.

Ramp checks for whether it should be checking for compatibility at all, and if it should - it checks the compatibility and adds a notice - leaving the rest of the Ramp behavior as it was previously. It's simple, easy, and it works.

Nauris added 3 commits November 2, 2018 10:59
Call it gutenberg_ramp_get_validated_gutenberg_load_path instead of gutenberg_ramp_validated_gutenberg_load_path
And update the wording in the message
Copy link
Member

@mjangda mjangda left a comment

Choose a reason for hiding this comment

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

🚢

@pyronaur pyronaur merged commit 7119efd into Automattic:master Nov 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants