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 time format with client side date parsing #415

Merged
merged 33 commits into from
Aug 22, 2023

Conversation

adamwoodnz
Copy link
Contributor

@adamwoodnz adamwoodnz commented Jul 18, 2023

Closes #413

This is an alternative approach to the server side implementation in #414

Pros:

  • date description parsing is done at the time of applying the format, rather than on page load on the server
  • no filtering of the_content

Cons:

  • if the date to be parsed by the format needs to be changed, or the post date is changed, the time format needs to be toggled off and on again to re-parse the changed date

UPDATE:
If the time description changes, the format will now be removed so that it can be reapplied and parsed.

Copy link
Contributor

@tellyworth tellyworth left a comment

Choose a reason for hiding this comment

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

Looks fine to me. I've only reviewed the code, not tested.

mu-plugins/blocks/time/index.php Outdated Show resolved Hide resolved
const linkElement = document.createElement( 'a' );
linkElement.setAttribute(
'href',
`https://www.timeanddate.com/worldclock/fixedtime.html?iso=${ datetimeISO }`
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noting for future possibilities: should we consider replacing timeanddate.com?

mu-plugins/blocks/time/src/index.js Show resolved Hide resolved
Copy link
Contributor

@ryelle ryelle left a comment

Choose a reason for hiding this comment

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

The editor side looks great, but it's not working on the frontend, and I think something's off with how the time is converted. I entered "Wednesday July 12 2023 at 15:00 EDT", but the datetime attribute is using the UTC value plus the timezone (2023-07-12T19:00:00-04:00).

mu-plugins/blocks/time/src/convert_times.js Outdated Show resolved Hide resolved
mu-plugins/blocks/time/src/convert_times.js Outdated Show resolved Hide resolved
adamwoodnz and others added 2 commits July 25, 2023 11:03
Co-authored-by: Kelly Dwan <ryelle@users.noreply.github.com>
@adamwoodnz
Copy link
Contributor Author

I entered "Wednesday July 12 2023 at 15:00 EDT", but the datetime attribute is using the UTC value plus the timezone (2023-07-12T19:00:00-04:00).

Hmm I hadn't tested with anything other than strings with UTC, as that's what I'd seen in posts. I'll take a look, ideally we could support more than that.

@adamwoodnz
Copy link
Contributor Author

For some reason this isn't working in my sandbox. The editor script loads and the format appears to be applied (inspecting the element in the editor shows the format), but when I save and load the frontend there is no format applied, and if I reload the editor the format is gone too. It's as if it's being stripped by some tag filter... @ryelle @tellyworth any ideas?

@ryelle
Copy link
Contributor

ryelle commented Jul 25, 2023

It's as if it's being stripped by some tag filter...

Ahh, I don't see time in the $allowedposttags list, so I think it is being stripped out. It's not a problem on single sites because those allow unfiltered_html for admins/editors. I think you can add time with the wp_kses_allowed_html filter.

@adamwoodnz
Copy link
Contributor Author

I think you can add time with the wp_kses_allowed_html filter

Thanks @ryelle this seems to work well

@ryelle
Copy link
Contributor

ryelle commented Jul 26, 2023

Front end view works now. I tried a few different formats, some copied from make/core, and it all worked well 👍🏻

I know you mentioned you can't reparse an updated date, but is it possible to auto-clear the format when the content changes? I think that's going to be confusing. I tried editing the day in a date and it turned into this:

Editor Frontend
Screenshot 2023-07-26 at 5 49 23 PM

@adamwoodnz
Copy link
Contributor Author

adamwoodnz commented Jul 27, 2023

Front end view works now. I tried a few different formats, some copied from make/core, and it all worked well 👍🏻

🎉

is it possible to auto-clear the format when the content changes?

Nice idea, I'll give it a try

@adamwoodnz adamwoodnz force-pushed the try/413-try-moment-time-format branch from 96088e5 to 82d9b2c Compare August 1, 2023 23:51
@adamwoodnz
Copy link
Contributor Author

is it possible to auto-clear the format when the content changes?

@ryelle I've implemented this now, please try again.

@adamwoodnz adamwoodnz requested a review from ryelle August 2, 2023 00:26
@StevenDufresne
Copy link
Contributor

I don't know if this is a valid use case, but I got some weird behavior:

  1. Successfully create a date,
  2. Change UTC to GMT to simulate a user error
  3. Notice you get double dates.

Screen Capture on 2023-08-07 at 09-32-53

@adamwoodnz
Copy link
Contributor Author

I got some weird behavior

Thanks for this testing. This is intermittent for me. I did manage to repro once, but after refreshing I can't repro again. I think we can merge and monitor.

Use inline styles in editor and remove on frontend
Handle post date being undefined and parse dates based on current time
@adamwoodnz adamwoodnz merged commit 4ddc904 into trunk Aug 22, 2023
2 checks passed
@adamwoodnz adamwoodnz deleted the try/413-try-moment-time-format branch August 22, 2023 02:30
@adamwoodnz adamwoodnz added [Format] Time Related to the Time format and removed [Block] New Block labels Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Format] Time Related to the Time format
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add support for [time] shortcode
5 participants