Open eliot-akira opened 7 months ago
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:
check
- Check any given valuefield
- Field value with dateacf_date
- ACF Date field (uses the formatY-m-d
to avoid the ambiguity of ACF's defaultd/m/Y
format)acf_date_time
- ACF Date/Time field
Could also add "to use a specific date format, pass the date field to the check
attribute" in that parenthetical or somewhere on that docs page.
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.
In terms of language design, I would have preferred to have both tags have the same default formats
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 If
tag works as expected, even for ACF date fields, but I'm now realizing that the correct expectation to set is probably instead that L&L will break or react unexpectedly as a result of ACF bad defaults. This is essentially the same breaking change we decided to introduce in 405fbf5. We can just let that happen and if people want to avoid the issue, they should adjust their field's return format in ACF. That way our little caveat/note in the docs would be written on the ACF docs page instead of the date conditionals page, which makes sense since this is an ACF quirk, not an L&L quirk. When we eventually support other custom field plugins like Pods, I think it'll make less and less sense to make plugin-specific allowances in the core functioning of L&L.
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:
ACF's default date return format,
d/m/Y
, is ambiguous and may present issues when passing its value to other tags, such as theDate
tag or date conditionals in theIf
tag. To avoid comparison difficulties, we recommend selecting an unambiguous return format in your field settings, such as the ISO-standardY-m-d
. Alternatively, theacf_date
field type'sformat
attribute or theDate
tag'sfrom_format
attribute can be used to safely convert the date to an unambiguous format.
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 If
tag, we could introduce the format
attribute, but make it work the same way it already does on the Field
tag, like <If acf_date=date_field format="Y" ends_with=0>
.
Alternatively, one thought I just had is that a way to avoid having this be a breaking change would be to deprecate the If acf_date
syntax altogether. Date conditionals would still be a thing, but they'd rely on the check
and field
attributes rather than offering anything ACF-specific. If someone wanted to achieve the logic above, they would just need to combine the general L&L features with the ACF-specific features that already exist, like <If check="{Field acf_date=date_field format='Y'}" ends_with=0>
.
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 If
tag docs page and move the ACF-specific stuff to the ACF docs page since at the moment, all of that stuff seems necessarily hidden on its own page.
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 Y
format to compare years). That way, plugin-specific considerations can be pushed out of the core and into the peripheral, in the integration code.
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.