-
Notifications
You must be signed in to change notification settings - Fork 3
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
ACF Date fields: Make default formats for Field and If tags consistent #95
Comments
Hmm, that's an unfortunate side effect of ACF's default date format. I don't think we'd need a special attribute like this: <If acf_date=date_field format=acf ends_with=2022> Because the user could always write this if they specifically wanted to use ACF's display format: <If check="{Field acf_date=date_field}" ends_with=2022> I'd be inclined to leave it as-is and make a note in the docs explaining this ACF-specific quirk. We already mention that some date formats present difficulty for comparison so we could reiterate that on the date conditionals page. Or we could simply write:
Could also add "to use a specific date format, pass the date field to the Does that make sense or am I misunderstanding the issue? |
OK, thanks for the feedback - I agree it's probably enough to have a note in the docs. In practice it's unlikely that people would want to compare dates in ACF's default format. In terms of language design, I would have preferred to have both tags have the same default formats, because this is inconsistent and surprising: <Field acf_date=date_field /> === d/m/Y (based on ACF field setting)
<If acf_date=date_field value=".."> === Y-m-d I wonder, is it the same for post publish date? <Field publish_date /> === d/m/Y (based on site setting)
<If field=publish_date value=".."> === Y-m-d Hmm, I'll have to check if this latter works as described. I don't remember putting in an exception for publish date, so it could be trying to compare a date formatted by site setting. Looks like this difference applies (or should apply) to all fields with date values, not only ACF date fields. |
After sleeping on it and reading your response, I think I've come around to your way of seeing this issue. At first, I was thinking we should fix ACF's format in favor of guaranteeing that our Plus, even if users pass the default ACF date format to date-related tags and attributes in L&L, it'll probably still work most of the time. It would only be when the date is between the first and twelfth day of the month that L&L could get confused and even then, I imagine there's a 50% chance it'll guess right. So my apologies for the complete 180, but I think the ticket here is just to let ACF's default date format cause unexpected issues in the rest of the L&L language and add a note to the ACF docs page in the date fields section:
I don't like that this is another breaking change to the way ACF's date format is used, but it feels more "correct" to be consistent with ACF's settings across the board rather than make L&L behave inconsistently to account for ACF's design oversight. It also results in less for us to keep track of since we can avoid adding ACF-specific exceptions to every tag and attribute that works with dates. If we wanted to include the consolidated shorthand syntax on the Alternatively, one thought I just had is that a way to avoid having this be a breaking change would be to deprecate the Whatever approach we take, I think I'm going to remove the standalone page for date conditionals on the docs and move the default date comparisons to the main At this point, I've argued both sides so feel free to go with whichever approach feels the most future-proof. |
Woof, I appreciate your thoughts considering this from various angles and pros/cons. Good point about how we would handle Pods and other custom field plugins in the future. Internally, the simplest and most consistent would be that the Field and If tags are always passed the same value, without making exceptions for plugin-specific quirks. For date-like field types, I think the default should be the predictable ISO 8601 format, and only convert to other formats for display purpose (Field tag) or special cases of comparison (like If tag with Ideally I'd like to apply the same to field-specific logic like post publish/modified date. Then all date-like fields can work the same across the board - regardless of where the value came from, WP core or a third-party plugin, and from any post type. <Field publish_date /> === Y-m-d H:i:s
<If field=publish_date value=".."> === Y-m-d H:i:s
<Field acf_date=date_field /> === Y-m-d
<If acf_date=date_field value=".."> === Y-m-d
<Field publish_date format=site /> === F j, Y (based on site setting)
<Field acf_date=date_field format=acf /> === d/m/Y (based on ACF field setting) |
<Field acf_date=date_field /> === Y-m-d This is kinda going back on the change that we made in the recent version release, but I think that was done before we (or at least I) realized that using ACF's settings by default would cause potential confusion with some of the date-related comparison features in L&L. Ultimately this comes down to the question: is it better to have ACF's own settings apply by default when working with ACF data, or is it better to have L&L ignore ACF's settings so its features work without needing to worry about ACF's settings? Easy to argue either way. I'll leave the final decision to you, though I'd be curious to get @GabrielGallagher or @juliacanzani to weigh in about which approach makes the most sense from a product design standpoint. |
The ACF date conditionals were affected by the new default date formats from ACF field setting (#89). I added an exception to bypass the formatting for values being passed to the If condition rules, which require a value that can be converted unambiguously for comparison.
The raw values are in the format as they're saved in the database by ACF, except for the Date field which is converted from
Ymd
that cannot be distinguished from UNIX timestamp.Y-m-d
Y-m-d H:i:s
H:i:s
Now the ACF date conditions work as they did before the change to the default date format.
However, this causes an inconsistency between what the
Field
tag and theIf
tag receive from ACF date fields. Possible solutions:Revert the
Field
tag to receive raw date value as before. Add a way to format it based on ACF field setting, like:Or, add a way for the
If
tag to format the date value based on ACF field setting, only when needed:The first option seems more consistent (both tags receive the same value) and backward compatible with 4.0 and before. I would also revert the change to apply default format from site setting, because that makes the value ambiguous again and different from what the
If
tag needs. But in this case, we'd need something likeformat=site
to specifically convert it according to site setting.The second option may be an acceptable compromise, where the
Field
tag shows what the user expects (formatted as ACF field setting); and theIf
tag gets what it needs, while allowing the user to compare against a formatted value as an exceptional case.@juliacanzani @GabrielGallagher @nicolas-jaussaud Please let me know if you have any preference or feedback.
The text was updated successfully, but these errors were encountered: