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

Proposed v3.3.0 - Major Changes #25

Closed
wants to merge 80 commits into from
Closed

Conversation

codejp3
Copy link

@codejp3 codejp3 commented Oct 4, 2022

Added PHP compatibility versions after testing:
https://wordpress.org/support/topic/php-8-0-support-17/

Added conditional check for non-object scenarios:
https://wordpress.org/support/topic/error-when-no-404-php-defined-in-theme/

Simple edits to remind me of what I want to do with this plugin.
Simple edit to remind me what I would like to do with this plugin.
Anticipate 404 errors and non-object queries where there is no $post->ID to get fields from and just return instead.
Removed my personal comments. Added PHP testing compatibility up to PHP 8.2.
Removed my personal comments. Added PHP testing compatibility up to PHP 8.2.
Option added to choose the source for font files, either remotely from Google, or save and serve locally on the hosting server.
@@ -81,6 +81,11 @@ function acft_enqueue_google_fonts_file() {

global $post;

// anticipate 404 errors or non-object queries where there is no $post->ID, just return.
if ( is_404() || !is_object($post) ) {
Copy link
Owner

Choose a reason for hiding this comment

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

I haven't tested the code yet but I think not enqueuing the Google fonts on 404 page could create issues for those users who have used ACF Typography in ACF Options. An example would be using the same header and footer as the rest of the site. We will likely need to do it differently. What are your thoughts?

Copy link
Author

@codejp3 codejp3 Oct 5, 2022

Choose a reason for hiding this comment

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

Yeah, I get that.

Don't merge this one. I've got it covered in a MAJOR pull request coming within the next day or two. It's everything I mentioned in this topic:
https://wordpress.org/support/topic/enqueue-fonts-styles-in-custom-function/

In order to accomplish those things, I ended up breaking your function for enqueue-ing Google fonts in half, using the first half as a new function, and it's called twice - to enqueue Google fonts, and by my new shortcode for inserting in-line font stylesheet links/styles code.

Part of that function separation involved code changes that should resolve this. It now follow this logic:

  • Function now allows for an optional $post_id to be supplied.
  • If no $post_id supplied, it'll try to grab it from inside the loop with global $post (what you originally had).
  • If still no $post_id, it'll try to grab it from outside the loop with global $wp_query.
  • If there's still no $post_id after all of that, it will only check for option fields, and return them as the "all_fields" variable without any array merging.
  • If a $post_id is present, it'll process post fields, block fields, option fields, and merge the arrays, and return the merged array as the "all_fields" variable (what you originally had).

Give me another day or two to finish up the edits. Not sure if it should be released as v3.3 or v4.0, but I'll leave that up to you.

removed old screenshot and added a new one that shows the updated Admin Settings Options Page with new options.
Added Changes to changelog for proposed changes in 3.3.0
Updated readme with new details about shortcode, additional features, and new screenshot.
fixed typo
Added info about new proposed release, new shortcode & filter hook, and available features
minor code cleanup for readability, and adding proposed new release version
minor code cleanup for readability
minor code cleanup for readability
*add new option for how to serve google font files - locally or remotely
*minor code cleanup for readability
*updated textdomain to be consistent with the rest of the plugin
*added description and link for getting Google Fonts API Key
*minor code cleanup for readability
* added new shortcode "acf_typography_stylesheet"
* added Filter Hook for new shortcode
* major overhaul of some existing functions, and addition of new functions
* fixed bug about trying to gt ID of a non-object
* code comments added
* minor code cleanup for readability
quick removal of commented code that is no longer used
@codejp3
Copy link
Author

codejp3 commented Oct 5, 2022

Here's the major overhaul, proposed version 3.3.0
I've done initial testing, but haven't thoroughly tested it yet.

Sorry it's a big one!

Includes the fix discussed previously for getting ID of non-objects, and all the new features mentioned in this topic:
https://wordpress.org/support/topic/enqueue-fonts-styles-in-custom-function/

I opted to leave out converting URLs to directory paths (what I need for my PDF generation function), because it's such an odd and uncommon thing and really doesn't belong in the core plugin. Instead, I included a Filter Hook within the new shortcode, so that I can alter the output for my specific use outside of the acf-typography plugin.

Overall changes:

  • [UPDATE] Fixed consistent textdomain usage for i18n translations.
  • [UPDATE] Made sure displayed strings are all i18n translation-ready.
  • [UPDATE] Added a few minor additional checks for some variables to prevent possible errors in the future.
  • [BUG] Fixed 'Notice: Trying to get property ID of non-object' when enqueueing font files on non-objects.
  • [NEW] Google Fonts can now be saved and served locally.
  • [NEW] Admin Settings Option to choose between serving font stylesheets and font files Remotely or Locally.
  • [NEW] Description and link provided for getting Google Fonts API key in Admin Settings page.
  • [NEW] Shortcode to print in-line stylesheet link or style HTML code (acf_typography_stylesheet).
  • [NEW] Filter hook for the new acf_typography_stylesheet shortcode to alter the output of the in-line stylesheet links/styles.
  • [NEW] Tested up to PHP 8.2
  • [UPDATE] Minor code cleanup for readability.
  • [UPDATE] Added new screenshot for new Admin Settings Page Options.

I'll send you a message after you've had a chance to digest this one. While making these changes, I also noticed an opportunity to support auto-enqueueing stylesheets for User Options/Profiles, Menus/Menu Items, Widgets, Attachments, Taxonomies, and Comments.....basically everything ACF supports adding fields/field_groups to. Depending on someone's use, some or all of these should be enqueueing stylesheets in wp-admin too. Want to talk to you directly about your interest in supporting these things, and the best way to go about it.

Cheers!

Previous changes make the readme.txt non-WP compliant. Just a few quick changes to fix my previous mistake.
Languages directory added to make plugin i18n translation-ready
added a translators comment to get rid of the nag message when generating the initial .pot file for this plugin
changed location of translators comment to get a successful .pot generation without nag messages
3rd time worked! translators comment added for successful generation of .pot file for translations
Initial .pot file generated to make plugin officially translation-ready and i18n standard compliant
added translation-ready change
added translation-ready message
added translation-ready message and changelog note
minor typo fix
Split up the large function handling saving/processing local stylesheets an font files into a handful of smaller functions. They're still bigger than I would like, but a lot better than a massive function handling it all.
Added updates for supporting ACF v6.
File is currently identical to acf-Typography-v5. All fields currently work fine under v6 as-is, but there may be visual/layout changes specific to the new ACF v6 interface applied to this file in the future.
Added checks for if ACF is installed and active. If not, admin message will show error message to install/activate ACF.
Minor changes to check if ACF installed/active, and only loading files that depend on ACF if it's actually present.
Changed order of google URL string generation to only strip last ';' when there's actually a value to strip it from.
Changed name of a function, and updated call to that function for the stylesheet shortcode to match.
Changed the logic for how/when to process locally served stylesheets and remote stylesheets. By default, remote will always take priority, unless local is explicitly defined. Affects shortcode for displaying in-line coe, and enqueueing for default WP stylesheet enqueues.
Small fix for including files with new ACF installed/active check. Local fonts were not being saved. Moved file includes back outside of ACF check.
@codejp3 codejp3 changed the title PHP Compatibility & Non-Object Scenario Check Proposed v3.3.0 - Major Changes Oct 8, 2022
Copy link
Author

@codejp3 codejp3 left a comment

Choose a reason for hiding this comment

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

Ready for Final Review.

Tested pretty extensively, but may need a few adjustments before being labeled "stable".

@mujahidi
Copy link
Owner

mujahidi commented Oct 8, 2022

@codejp3 Thank you so much for your hard work and contribution to this repository. I have a request. Can you please verify first before pushing your changes to this pull request? With so many commits, I think we ought to rebase them.

I still haven't been able to look at your change requests, and hopefully, I will do that in a couple of days.

It is possible I do not release all the changes in v3.3.0. We'll have to check and validate that nothing breaks thoroughly. I am looking to release multiple versions and patches. It is up for discussion though.

Thanks once again.

@codejp3
Copy link
Author

codejp3 commented Oct 8, 2022

@mujahidi What are your thoughts on the best way to handle these changes. There's more I'd like to add, but I want to get these current changes in the base core code before expanding even more.

We could break down each "type" of change into their own minor releases. Trickle the changes out one at a time.

I think that would work for the majority of changes, EXCEPT for the major change of saving/serving stylesheets/font files locally.

If this makes sense, I'll start a new branch of 3.2.4, and add a few small changes and push them your way. Then 3.2.5, 3.2.6 and so on. A little at a time. Let me know!

@codejp3 codejp3 deleted the branch mujahidi:master October 12, 2022 19:43
@codejp3 codejp3 closed this Oct 12, 2022
@codejp3 codejp3 deleted the master branch October 12, 2022 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants