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

Add human modifier for date and time fields #2131

Conversation

omarkasem
Copy link
Collaborator

@omarkasem omarkasem commented Sep 5, 2024

  • FIxes Add support for :human merge tag modifiers on all Date fields #1990
  • I had to edit the GravityView_Field_Date class because it was overriding the human modifier by running the function again without the new arguments and i couldn't remove the filter gravityview/merge_tags/modifiers/value as it was happening after the human modifier function.

💾 Build file (cb02ebd).

@omarkasem omarkasem linked an issue Sep 5, 2024 that may be closed by this pull request
@omarkasem omarkasem self-assigned this Sep 5, 2024
@mrcasual
Copy link
Collaborator

mrcasual commented Sep 5, 2024

@omarkasem, code looks good, thanks. Could you please add a unit test for this?

@Mwalek, please test.

@mrcasual mrcasual requested a review from Mwalek September 5, 2024 16:59
@Mwalek
Copy link

Mwalek commented Sep 5, 2024

@omarkasem I think there is a problem with the handling of future dates (see image).

Screenshot 2024-09-05 at 7 52 53 PM

Try site: https://diy-issue-248964a.try.gravitykit.com/display-human/

@omarkasem
Copy link
Collaborator Author

@Mwalek Thanks for noticing, It's fixed now.
@mrcasual I also added extra tests for future dates and times.

@mrcasual
Copy link
Collaborator

mrcasual commented Sep 7, 2024

@Mwalek, please retest and also try with different timezones.

@Mwalek
Copy link

Mwalek commented Sep 8, 2024

@omarkasem @mrcasual future dates are now displayed correctly.

As for the timezone, it doesn't matter what is set in WordPress (settings > general), it looks like the server timezone (UTC) is employed. This applies to both comparisons of past and future dates.

In the image below, I have the WordPress timezone set to UTC+2, my timezone, so if the human time difference were applying this setting, I would expect to see the next day starting in 2 hours instead of 4 (see image).

  • Future dates are displaying “{time} ago” instead of “from now”
  • Timezone offsets are not handled properly
Screenshot 2024-09-08 at 10 01 28 PM

@mrcasual
Copy link
Collaborator

mrcasual commented Sep 8, 2024

@omarkasem, you may find this relevant: 2850c35.

@mrcasual
Copy link
Collaborator

@omarkasem, please see comments above.

@omarkasem
Copy link
Collaborator Author

@Mwalek Thanks for the detailed comment, I think the last commit fixed the timezone issue.

@Mwalek
Copy link

Mwalek commented Sep 24, 2024

@omarkasem @mrcasual I confirmed that the timezone issue is fixed in the latest commit.

@mrcasual mrcasual merged commit c0f9743 into develop Sep 24, 2024
@mrcasual mrcasual deleted the issue/1990-add-support-for-human-merge-tag-modifiers-on-all-date-fields branch September 24, 2024 13:22
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.

Add support for :human merge tag modifiers on all Date fields
3 participants