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

ACF Support #41

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

jonny-bull
Copy link
Contributor

Adds an ACF filter for Flagpole flags

Screenshot 2021-08-02 at 14 50 13

Allows users to show or hide ACF field groups based on a feature flag, or combination of multiple flags.

@jamesrwilliams
Copy link
Owner

Hey @jonny-bull! Thank you for your PR and the time you spent on this feature, it's very much appreciated. 🥇

Just testing the your new branch locally and initially I didn't have ACF installed and I get this fatal error.

Fatal error: Uncaught Error: Class 'ACF_Location' not found in /flagpole/includes/acf/class-acf-filter.php:21

Can we set this up in a way that checks for the plugin's presence before initializing just-in-case any users don't want to use it with ACF?

Probably a wider question this also, the includes/acf/class-acf-filter.php is using PHP v7.1+ features I believe, do we want to make the PHP bump a requirement, and change the minimum PHP version in flagpole.php#L17 to match? Maybe just a major release feature?

@jonny-bull
Copy link
Contributor Author

Apologies for the delay on responding.

I've updated the code to support PHP 5.6. I wouldn't be against bumping the PHP version up to 7.1 but wouldn't want to break it for any current users. Waiting for a major release sounds like a sensible option.

Also fixed that bug with the ACF_Location class. It should behave a lot better now.

@jonny-bull
Copy link
Contributor Author

@jamesrwilliams have you had chance to look at these PRs?

I've noticed my PRs fail the new code analysis check but, from what I can make out, most of the code in this plugin fails those checks. Following existing conventions within the plugin is largely no longer an option and amending existing functionality can lead to some fairly major rewrites.

There's also no documentation of what the checks are, as far as I can see, so it's hard to make sure I'm following the required standards locally.

So:

  • what are these checks?
  • are they here to stay?
  • are they currently configured to the correct severity?

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