- Issue created by @Odai Atieh
- Status changed to Needs review
over 1 year ago 10:23am 18 April 2023 - 🇨🇦Canada mandclu
Thanks for identifying this. I agree that the timezone token should be treated as a valid character, though I am more than a little puzzled by the configuration in your screen capture. Why would you want the timezone to show as the date instead of as part of the time?
- 🇯🇴Jordan Odai Atieh Amman
I needed to display only the timezone to follow the design and requirements for a project.
- 🇨🇦Canada mandclu
If you had added the timezone to the time format, it should have been automatically deduplicated. Why wouldn't this be sufficient?
As I continue to think about this, I'm not sure I agree that timezone tokens should be considered as valid by the
rangeDateReduce()
method. - 🇧🇪Belgium dieterholvoet Brussels
I'm having the same issue setting PHP Time Format to
W
. - Merge request !54Issue #3354858: Warning: Trying to access array offset on value of type null → (Open) created by dieterholvoet
- 🇧🇪Belgium dieterholvoet Brussels
Regardless of whether or not certain tokens should be allowed, we should fix the warning.
- 🇧🇪Belgium dieterholvoet Brussels
Okay, but as long as the 3.x versions are supported we should probably also fix it there.
- 🇺🇸United States codechefmarc
This was what we were looking for too, however, we also needed the `c` format for "ISO 8601 date" so here is my updated patch that also includes that one.
- 🇺🇸United States codechefmarc
I just updated to version 4.1-rc6 and still had this issue. I didn't know of a good way to add a MR against that version since this ticket is meant for 3.7. So, I'm attaching a patch that will fix this for 4.1-rc6.
- 🇨🇦Canada mandclu
I'm really tempted to close this as "won't fix". If the goal is to output the timezone separately, that would be much better done using a preprocess function in your them. Simply put, "T" is not a day or month token, and adding it is going to break more typical use cases this code was designed for.
The c token is an interesting use case. I can see the argument for saying it should be supported, but I'm not sure this is the way to do it.
I've dropped support for the 3.7.x branch, so moving this to the 4.1.x branch.
- 🇧🇪Belgium dieterholvoet Brussels
Looks like the MR and patches have diverged. I also added the W format character in the MR, but seems like that one is missing from the patches. What are your thoughts about that @mandclu?
- 🇨🇦Canada mandclu
I've never needed to use the W character. Can you give me an example of where that would be useful? I'm skeptical that this is useful in range formatting.
Thinking out loud, another option might be to disable the validation completely if the Smart Date format has deduplication turned off.
- 🇧🇪Belgium dieterholvoet Brussels
We used this in context of a time tracking tool, a performance entity has a date range field and in certain views we had to display a week number column/filter for billing reasons.
- 🇺🇸United States codechefmarc
Would like to mention that we added
c
as a format that we are using and I hope this makes it into the next version. If this doesn't make sense, let me know if we should use the preprocess method instead. Thanks! - 🇨🇦Canada mandclu
Here's what I was thinking, in MR !107. Instead of adding to the validation checks things that don't make sense there, remove validation entirely if deduplication has been turned off for a format. In my initial testing this works for the "c" and "W' formatting strings.
- 🇺🇸United States codechefmarc
Hi, I just tested MR107 on our setup, and I'm still getting the error:
Warning: Trying to access array offset on value of type null in Drupal\smart_date\Plugin\Field\FieldFormatter\SmartDateFormatterBase::rangeDateReduce() (line 661 of modules/contrib/smart_date/src/SmartDateTrait.php).
We aren't using the normal Drupal formatter here, we're using Layout Builder and getting the data in a custom block, not sure if that helps. For now, I'll stick to the patch from before and/or explore a preprocess to see if I can fix. Any pointers where I need to target the preprocess? Thanks!
- 🇨🇦Canada mandclu
@codechefmarc did you disable "Reduce output duplication" on the Smart Date format you're using? I also just added a commit to that MR that may help.
- 🇺🇸United States codechefmarc
@mandclu - thank you! That did it! I'm now running with the patch from MR 107 and that "Reduce output duplication" and that worked - no more errors.
- Status changed to Fixed
7 months ago 12:35pm 3 May 2024 - 🇨🇦Canada mandclu
Great. Merged in the changes from MR 107. Thanks for everyone's work on this.
Automatically closed - issue fixed for 2 weeks with no activity.