Skip to content

Time Zone Support (#1344)#2633

Closed
JeremyKeusters wants to merge 15 commits intoYOURLS:masterfrom
JeremyKeusters:issue1344
Closed

Time Zone Support (#1344)#2633
JeremyKeusters wants to merge 15 commits intoYOURLS:masterfrom
JeremyKeusters:issue1344

Conversation

@JeremyKeusters
Copy link
Copy Markdown
Contributor

@JeremyKeusters JeremyKeusters commented Mar 28, 2020

This is a pull request for an implementation of time zone support and should resolve #1344 .

A new config parameter was added to the sample-config.php file called YOURLS_TIMEZONE. When this value is left empty, the value of YOURLS_HOURS_OFFSET is used, which allows for backward compatibility. This allows to keep using YOURLS_HOURS_OFFSET and avoids the need for all users to add the new config parameter YOURLS_TIMEZONE to their config file.

I first experimented with DateTime objects, but eventually went for a solution that calculates the offset of the timezone such that most of the original code can be kept, which reduces the risk of bugs.

Feedback and reviews are highly welcome, PHP is not my main language and I'm not sure either about the performance impact of my fix. Thanks!

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

My build is failing, but I'm supposing this has something to do with #2627 since it's failing on NextDecimal_Tests.

@My1
Copy link
Copy Markdown

My1 commented Mar 29, 2020

couldnt one maybe instead of having the same time stuff written all over the place instead have one time utility function which globally does time and is just called from everywhere needed?

would make it more maintainable as you dont have a metric ton of instances of the code you have to manually replace and stuff

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

JeremyKeusters commented Mar 29, 2020

Might be cleaner yes @My1 . There's currently 4 parts of code in 3 files that I had to mainly edit. They generally require a timestamp, but they are formatting all in a different way.

Eg. in functions_html.php, the PHP date function is used to generate a formatted date out of an epoch timestamp. In yourls-infos.php around line 375, yourls_date_i18n is used to format an epoch timestamp. However, around line 170 in the same file, we need the offset (in hours) to generate the SQL query. Finally, in functions-l10n.php around line 704, gmdate() is used to convert an epoch timestamp.

So there's 3 parts were we need a timestamp and 1 part where we need the offset in hours.

Now that I'm looking back at my code, I think that a time utility function would indeed be great since the whole if/else structure is basically duplicate. The utility function should have an epoch timestamp as input and output a 'timezoned timestamp' (based on the current if/else loop). Should I add this utility function to the functions-formatting.php file? Or is there a more suitable location for it to be?

Will start working on it already and put it in functions-formatting.php for now!

@My1
Copy link
Copy Markdown

My1 commented Mar 29, 2020

does this thing deal with local times in the db? lol for formatting one could maybe do a date style implementation, like for example yourls_time($format,$unixtime) and you could get the offset from time in hours and seconds using O, P and Z, so even that would be covered (even if you do neeed to do some extra work to change to a float hour value.

also if done well one could set a default format somewhere to have it nice and clean everywhere

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

Well, in the DB everything is saved in GMT time. In the code itself however, epoch timestamps are used.

About that general date style implementation: this is probably a good idea when rewriting the core, but I think that will be a bit intrusive for now IMO.

@My1
Copy link
Copy Markdown

My1 commented Mar 29, 2020

oh okay it just sounded a bit weird that the time offset was needed for something in the db when everything is GMT anyway and when in the code unixtime is used.

maybe if the core gets a rewrite all datetime things could just be unix timestamp in db and not needing to hurl around with ANY formatting exceprt for display

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

JeremyKeusters commented Mar 29, 2020

True! Currently working on a new patch, will push soon. Looks a lot cleaner with a single yourls_get_timezoned_timestamp and a yourls_get_offset.

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

To summarise: time zone support was rewritten to use one out of the 2 new functions in functions-formatting.php. One function outputs a timezoned epoch timestamp. The other outputs the offset in hours.

@ozh
Copy link
Copy Markdown
Member

ozh commented Mar 29, 2020

Interesting ! Much thanks for this. I'll have a closer look soon

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

My pleasure, thanks! :)

@PopVeKind
Copy link
Copy Markdown
Contributor

Pros & Cons

Let me first state that I am 100% in favor of migrating YOURLS from offset based to Timezone based. If done right I see some advantages. If done wrong I see additional problems. That said, I think we should look at the Pros and Cons of this PR.

Pros

It takes a step toward using Timezones. @JeremyKeusters made a good first effort.

Cons

Added Constant

/** Server timezone.
 ** Please use one of the listed timezones in the PHP documentation: https://www.php.net/manual/en/timezones.php .
 ** If left empty, YOURLS_HOURS_OFFSET will be used. */
define( 'YOURLS_TIMEZONE', '' );

Ozh has many times said we need fewer constants, not more. Because constants are problematic, and hard for plugins to change.

Many YOURLS Admins have no more PHP knowledge than running a WordPress site that someone else set up. Asking them to go to a php.net site is not likely to get a well-formed answer in all cases. Not to mention typos and copy/paste errors made by noobs and PHP experts alike. This creates more issue reports.

Solution: Store Timezone as a DB Option. No constant is needed.
Solution: Use the PHP Timezone functions to generate a pulldown list to select a Timezone (like WordPress does).
Solution: In addition, when PHP is updated, the Timezone list and pulldown menu are also updated automatically.
Solution: The Offset is used correctly by a minority of sites. this Timezone change code could be placed in a plugin (for the minority who want it) or in the Admin section. This would also be noob friendly.

Unnecessary Constant

/** Server timezone GMT offset */
define( 'YOURLS_HOURS_OFFSET', 0 );

I have used a part of YOURLS as a run one time only. This could be used as a conversion program without changing YOURLS performance.

Solution: The same one time only script could be expanded for new sites into a 4-minute install!
Solution: Evaluate YOURLS_HOURS_OFFSET, if it exists, and then store a timezone in a DB Option on a one time only bases for the conversion of existing sites.
Solution: Retire YOURLS_HOURS_OFFSET

Code Bloat & Two Systems
This adds code everyone must run, and it leaves two systems where only one is needed.
Solution: Retire and remove code for HOURS_OFFSET and automatically convert it to a Timezone.

@My1 this would unify timezone stuff on the DB Option which YOURLS already handles.

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

Thanks for the feedback!

Many YOURLS Admins have no more PHP knowledge than running a WordPress site that someone else set up. Asking them to go to a php.net site is not likely to get a well-formed answer in all cases. Not to mention typos and copy/paste errors made by noobs and PHP experts alike. This creates more issue reports.

If a user can create a MySQL login & a database and put those values in a config file together with the MySQL hostname, he/she will also be able to go to a simple website and look for their official timezone IMO? I think you're slightly underestimating the average user here. There's no way that you can set up YOURLS without having at least some technical knowledge/feeling. If really needed, we can add some extra explanation and/or an example: eg. Europe/Brussels.

Solution: Store Timezone as a DB Option. No constant is needed.

This would require a whole re-do of the current config options (and put them all in the DB), you're not just going to put only the timezone option in the DB. You'll also need an admin interface to configure this and a full-fledged installer in which you can configure the setting. This would indeed be nice, but this is something for the long run, maybe for 2.0. It would be nice if Time Zone support could come in before 2.0, since it's an issue that's been open since 2013.

Solution: Use the PHP Timezone functions to generate a pulldown list to select a Timezone (like WordPress does).
Solution: In addition, when PHP is updated, the Timezone list and pulldown menu are also updated automatically.

Again, you would need to create an admin interface to show this pulldown list.

Solution: The Offset is used correctly by a minority of sites. this Timezone change code could be placed in a plugin (for the minority who want it) or in the Admin section. This would also be noob friendly.

Following your reasoning from above, you would need to move the offset too to the DB and all my remarks would be applicable again. How is an offset more noob friendly than a timezone? With offset, I'm pretty sure that most of 'your average users' need to look up first how many hours they are differing from GMT, maybe some even need to lookup what offset and what GMT means. Then the confusion comes when they read that they are for example +7 hours from GMT. "Should I write +7 or 7? Or should I write '7 hours'?" Also: imagine that they look up their GMT offset and see that it's GMT+2 in DST and GMT+1 in standard time? "What offset should I take?"

@PopVeKind
Copy link
Copy Markdown
Contributor

PopVeKind commented Mar 30, 2020 via email

@PopVeKind
Copy link
Copy Markdown
Contributor

@JeremyKeusters Hey Jeremy,

They say a picture is worth a thousand words, so I had a few minutes today and I thought I would paint you one in PHP.

The Timezone dropdown menu I'm going to show you took 5 minutes to write. It has 4 lines of code and 178 Characters (including spaces!).

I want you to try to look at this through the eyes of a real noob.

Method One:

/** Server timezone.
 ** Please use one of the listed timezones in the PHP documentation: https://www.php.net/manual/en/timezones.php .
 ** If left empty, YOURLS_HOURS_OFFSET will be used. */
define( 'YOURLS_TIMEZONE', '' );

That has lots of ways for a real noob to mess up.

Or This...

Method Two:

Timezone Pulldown Example

Impossible for a real noob (or anyone) to break YOURLS with this.

Note: this pulldown is generated by PHP so it is always correct. Likewise, I have added no styles to make it look good, I'm sure we could do better!

@PopVeKind
Copy link
Copy Markdown
Contributor

@JeremyKeusters

I have been looking at your PR code and I think it shows real promise. I have a few questions:

1. Refactoring to Seconds
By the time it is actually used, YOURLS_HOURS_OFFSETis converted to seconds using either:
$offset * 3600
OR
$offset * 60 * 60.

Your code takes the timezone offset, already in seconds, and converts it to hours.
$offset = $datetimezone->getOffset(new DateTime("now", new DateTimeZone("GMT"))) / 3600;
This must then be reconverted into seconds.

Wouldn't it be cleaner to do one conversion of YOURLS_HOURS_OFFSET into seconds first and do a little code refactoring to do everything else using seconds?

2. Avoid Adding Constants
You have this:
define( 'YOURLS_HOURS_OFFSET', 0 );
And you ADD this:
define( 'YOURLS_TIMEZONE', '' );

Why not combine them?

For a Philippine example:
define( 'YOURLS_HOURS_OFFSET', '8' );
OR
define( 'YOURLS_HOURS_OFFSET', 'Asia/Manila' );

Where it is used, test if it is an integer (hours offset) or text (timezone). If '0' or empty then set timezone to UTC.

3. This would indeed be nice!

This would require a whole re-do of the current config options (and put them all in the DB), you're not just going to put only the timezone option in the DB. You'll also need an admin interface to configure this and a full-fledged installer in which you can configure the setting. This would indeed be nice...

It might surprise you to know that this was already written in 2018. the site, under test for about 18 months, moved and refactored each of the constants in config.php and put these in DB Options. It has full automatic conversion, therefore it is noob-friendly. Added features include Multisite (one set of files, multiple domains, multiple sets of keys), Full-Fledge Easy 4 Minute Installer, co-location of WordPress and YOURLS in the root directory of a single domain, without any restriction to either program (People say this cannot be done, but it runs just fine with only two copy/past lines of code!)

The Problem was, the PR is Huge!

After a long discussion with @ozh he advised me to break this into small PRs that focused on one thing at a time. I had just started that when I had a stroke, and a few months later another stroke. For the last year, I have had to focus on higher priorities like learning to talk, walk, and eat a completely different way.

The point is, that code still exists, and I could extract the pieces for the timestamp project. So we could do timestamp first and "nice" could start in March or April 2020!

Would you first look at 1. and 2. above. that would make 3. easier.

@ozh
Copy link
Copy Markdown
Member

ozh commented Apr 1, 2020

I think I'll build on @JeremyKeusters patch, to leave in core functions like his yourls_get_timezoned_timestamp(), and add a core plugin to change timezone for those interested.

Benefits:

  • no extra constant added, and we can even deprecate YOURLS_OFFSET
  • doesn't change anything to people who don't care about timezones (99% of users I think)
  • people who care have a convenient interface in a plugin

@My1
Copy link
Copy Markdown

My1 commented Apr 1, 2020

that is a good way. offer a standardized way to get time and let plugins do the rest.

btw if doing such a thing maybe add a way to change the timestamp format into the function as well so people can have time formats they want (by using the appropriate plugin).

Just want to check if I know how to push from my local repo to Jemery's repo. Yeat, I'm a git noob :)

[skip ci]
@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

JeremyKeusters commented Apr 1, 2020

Great solution balancing both my PR as well as PopVeKind's remarks. Let me know if I can help with the plugin, I currently have time to work on stuff like this.

We can as @My1 suggests make this bigger and change the yourls_get_timezoned_timestamp() to output a string, so that you can standardise all the time display throughout YOURLS. Issue with this is that you probably want a different format for the created column in 'list of URLS' versus for the text 'Short URL created on January 29, 2020 @ 12:44 pm' that is in the statistics of a URL.

@My1
Copy link
Copy Markdown

My1 commented Apr 1, 2020

Issue with this is that you probably want a different format for the created column in 'list of URLS' versus for the text 'Short URL created on January 29, 2020 @ 12:44 pm' that is in the statistics of an URL.

true enough although some forums or other CMSes or whatever I saw offering 2 options, long and short date format (ignoring the fact that delta format [aka, "x time units ago"] also exists).

and for example the idea would be that yourls would have one long and one short default time format like
"2020-12-31 18:30" (I chose ISO-style as it's the most clear format which generally cant be misinterpreted, like others with dots or slashes where month and year can be swapped depending on region making it chaotic) as short and "December 31, 2020 18:30" as long format, and the function call could be basically

get_timestamp([$format=TIME_FORMAT_SHORT],[$time=now]) (just as a generic idea, names are just for easy understanding)
where format can be either long or short (by constant as integer) or datestring.

if datestring is set, ignore below (like when a call to the timezone needs a specific time format is needed at some place for whatever reason), otherwise continue

if no datestring is set let's just go for short as an unintentional long can break display, an unintentional short breaks less

if the format is one of the constants and a display format for that has been set by plugin or config or whatever, use that,

if no display format is set for the given type, use the defaults

@PopVeKind
Copy link
Copy Markdown
Contributor

What I used for my dropdown. Not pretty but maybe it will help.

echo "<select>";
$tzlist = DateTimeZone::listIdentifiers(DateTimeZone::ALL);
foreach($tzlist as $value) echo "<option value=". $value .">". $value ."</option>";
echo "</select>";

@ozh
Copy link
Copy Markdown
Member

ozh commented Apr 2, 2020

Only the timezone will be stored in the DB.

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

Thanks for the additional explanation @ozh !
Will try to work on it further today/tomorrow.

What I used for my dropdown. Not pretty but maybe it will help.

echo "<select>";
$tzlist = DateTimeZone::listIdentifiers(DateTimeZone::ALL);
foreach($tzlist as $value) echo "<option value=". $value .">". $value ."</option>";
echo "</select>";

I already use this, but thanks.

Instead of storing both the offset and the timezone in DB Options...

As @ozh already said, we are only storing the timezone in the DB. Offset will be completely deprecated (in the future). When not using the timezone plugin, it will simply use GMT time.

@ozh
Copy link
Copy Markdown
Member

ozh commented Apr 2, 2020

@JeremyKeusters I propose the following code to display the dropdown: https://gist.github.com/ozh/9981e14eff488e6e071913b817a18fbc

This needs to be improved : output should be translatable (ie yourls__('strings') instead of the plain echo or print) and something to have the saved option selected in the dropdown

@PopVeKind
Copy link
Copy Markdown
Contributor

@ozh That is an interesting pulldown list. I have suspected that UTC+08:00 was an undocumented but a valid timezone.

yourls__('strings') is an interesting idea, perhaps PHP has a language option? yourls__('strings') could be worked into the code I presented above.

Manual Timezones are not needed and cause more Issue reports.

Although this pulldown is better than "type in your timezone"...

Are Manual timezones needed or even desired?

This dropdown creates an option for every imagined timezone that is 15 minutes apart. However, many or most of these are not used by anyone. WordPress uses a custom list of manual timezones and updates this in the WordPress code as needed. However, Is this desired?

The first half of this dropdown presents the same information as the one I used (see the example page. Notice these are sorted by region. People complain about the WordPress dropdown because as you scroll through it, you lose track of what region you are in (the regions drop off as you scroll). Do we want to be just like that?

The example page contains all the timezones that are used in the world, maintained by PHP using the official IANA list. Why are "Manual timezones" needed? They only confuse people (noobs) and they do not do DST/Summer Time. They have the same problem that offset has. The offset would accept a value of 5.75 (same as UTC+05:45) but No DST adjustment.

Supporting unnecessary choices (Manual Timestamps) has only caused more WordPress questions, issues, and bug reports that needed to be answered. For example:

Issue: "I found a bug in WordPress. I live in Los Angelus, California and correctly selected the manual timezone of UTC-08:00 on the dropdown list. However, my time is still wrong."

Answer: "Don't use Manual Timestamps. Use America/Los_Angeles instead."

Problem: UTC-08:00 does not have DTS. America/Los_Angeles does have DST. If for a strange/unbelievable reason someone did need UTC-08:00 without DST, they would know to select Pacific/Pitcairn.

Do what you like, I won't complain (any dropdown menu is far better than a constant). However...
I strongly urge you to not include manual timezones in your dropdown menu. These will only increase issues and make additional work for the Core Team.

@ozh
Copy link
Copy Markdown
Member

ozh commented Apr 2, 2020

Selecting UTC and not having DST is the expected behavior. DST are regional choices. Selecting UTC is made available for specific cases when people want to pick a UTC offset but not comply to a timezone DST setting. This will be requested by people who care about time offset.

@ozh
Copy link
Copy Markdown
Member

ozh commented Apr 2, 2020

@JeremyKeusters if I understand correctly, this PR will also need a change in every SQL query made where a timestamp is stored.

  • Current behavior: store server timestamp as current date, and output with a time offset (eg my YOURLS instance is hosted in the USA but I live in Europe: add 7 hours)
  • New behavior if we manage timezones: store UTC timestamp, regardless of server time, and output that timestamp, plus or minus time offset as per timezone setting if applicable.

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

@ozh last time I checked, I noticed that the timestamps are already stored as UTC time? So no change is needed in the current SQL queries?

The offset I was talking about is for a SELECT query, where you select all the clicks from the last 24 hours eg.

@ozh
Copy link
Copy Markdown
Member

ozh commented Apr 2, 2020

@JeremyKeusters totally possible. I'm a true SQL noob :)

@PopVeKind
Copy link
Copy Markdown
Contributor

@ozh If you want me to leave the YOURLS community just ask and I'm gone. Otherwise please consider who, under what circumstance, would use UTC+02:15 and why that outweighs confusing Noobs with unnecessary extra options and code bloat that will never be used?

Selecting UTC (offset) is made available for specific cases when people want to pick a UTC offset but not comply to a timezone DST setting. This will be requested by people who care about time offset.

What Specific Case?

PHP has included "ALL" offsets, those with and those without DST.

I just wonder if anyone can give me a real-world example of why anyone would want to use this, within YOURLS? I'm not trying to be difficult, I'm just trying to understand. The DST is only applied to countries that have laws requiring it. That is why UTC+03:15 is not included in the PHP "ALL". Some countries do not use DST, like the Philippines.

Asia/Manila is equal in all ways to UTC+08:00

For that one person doing something really weird, there are alternative timezones that can be selected from the PHP "ALL" list. It seems like a lot of extra work and code bloat, in all YOURLS installs, just to output something that will not be used and will confuse Noobs.

Or UTC-Offsets could be put in a user-provided UTC-Offsets Plugin, for that one person doing something super weird.

Have we changed from helping Noobs to confusing them?

Just because we can make this list that won't actually be used, does not mean we should do it.

Or is it just because you don't like my long explanations? I know ozh and I did not see eye to eye in the beginning, till I started understanding his reasoning. But is that a good reason to add code bloat, that likely never will be used, and will confuse noobs?

All I ask is to be considered like everyone else. I ask anyone to explain why this core plugin code bloat, will actually (not hypothetically) be used by you in your YOURLS install? That seems a fair request for a real community.

@PopVeKind
Copy link
Copy Markdown
Contributor

2nd Thought: May we leave this plugin as a contributed plugin? So this unnecessary and unused UTC-offset code bloat, as a core plugin, does not have to be downloaded and installed on every YOURLS install?

@My1
Copy link
Copy Markdown

My1 commented Apr 3, 2020

well but one does have to note that finiding a named time zone that is with/without DST for any given offset can be a pain. so the ability to just specify a dumb offset may not be too dumb.

@ozh
Copy link
Copy Markdown
Member

ozh commented Apr 3, 2020

@PopVeKind :

  • "If you want me to leave the YOURLS community" because I disagree with your point on manual UTC is drama queen stuff I don't want to spend time on. The "code bloat" you mention is 15 lines of code that slightly extend the one feature we're dealing with. I don't consider it problematic to ship this with every YOURLS zip. I'm sorry if that bugs you beyond acceptable.
  • "Or is it just because you don't like my long explanations?" I have already expressed that, indeed, I find your 30 line replies unnecessary long and verbose. The signal to noise ratio is too low for me and I'm sure I've skipped or overlooked potentially interesting ideas from you because they're diluted. TL;DR.

Open to discuss this off site if you want, I'm at ozh at ozh dot org. Let's keep in mind that every comment here is mailed to 300+ people so we should keep it concise, on topic and constructive.

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

@JeremyKeusters I propose the following code to display the dropdown: https://gist.github.com/ozh/9981e14eff488e6e071913b817a18fbc

This needs to be improved : output should be translatable (ie yourls__('strings') instead of the plain echo or print) and something to have the saved option selected in the dropdown

Will try to implement this code! I like the fact that the time zones are grouped in the dropdown. I'm however not fond of the AM/PM time between brackets. Do we need the 'actual' time altogether? The time doesn't update 'live', so it takes the time that the user opened the page, which might be confusing? I would just leave it out, not sure what you/others think about this?

I mean, I suppose that people know in which timezone they live based on their city? What we could do is show the time that will be used beneath the dropdown once the user has selected a time zone. Something like this:

Without a time zone selected:

Select your time zone: [Dropdown](Select Time Zone)[/Dropdown]

Date and time will be displayed as follows:
April 3, 2020 - 16:19

With a time zone selected (Europe/Brussels in the example is GMT +2)

Select your time zone: [Dropdown]Europe/Brussels[/Dropdown]

Date and time will be displayed as follows:
April 3, 2020 - 18:19

@ozh
Copy link
Copy Markdown
Member

ozh commented Apr 3, 2020

Sounds fine to me to have the time shown below the dropdown. While we're at it, it would be very nice as @My1 suggested to have a way to customize date display, for instance a field where you enter a custom date/time format or select from a couple choices. WordPress does it nicely I think.

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

I'm currently experimenting with the code you provided in combination with a display of the current time using the selected timezone. I'm however quite a noob in jQuery, and I'm not sure how to combine PHP and jQuery. I suppose that I need jQuery for updating the time as soon as the time zone is changed/selected in the dropdown (we want the time to be displayed live, not requiring the user to reload the page), but I'm not sure of the compatibility between the time zones in PHP and the time zones in jQuery/Javascript. Also, we can't use the UTC options with this current solution.

GitHub gist

Demo

@My1
Copy link
Copy Markdown

My1 commented Apr 3, 2020

personally I would prefer a normal settings form -> save approach, as you can actually see whether or not it properly saved and it not leaving you in a weird state

I am not even aware how much this thing actually relies on js compared to PHP

@ozh
Copy link
Copy Markdown
Member

ozh commented Apr 3, 2020

@JeremyKeusters a way to simplify the JS would be to generate the dropdown list with a data attribute with the time, eg
<option name="Region/City" data-current="03/04/2020, 23:53:07">City</option>
and then getting this data tag with JS. Wouldn't update live, but relies only on PHP

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

JeremyKeusters commented Apr 4, 2020

A new push in which I implemented the new time zone dropdown that @ozh provided, but left out the 'live' time indication for now. Also implemented the 'wordpress' alike Date and Time format options. The date and time format options are saved in the database, but not used yet. Time zone is only used for the main page which lists all urls.

Small screenshot for those who just want to have a quick idea without running the code:
Schermafbeelding 2020-04-04 om 21 33 47

@ozh How do you see the further implementation of the date and time format options?

  • Should we continue filling the functions-formatting.php file or should we maybe create a new functions-date-time.php?
  • I suppose we will call a function at all places where we need to display a date time that will return a string that is formatted following the options?
  • What should we take as default formatting option?

@ozh
Copy link
Copy Markdown
Member

ozh commented Apr 5, 2020

@JeremyKeusters Looking neat!
Regarding functions-formatting.php vs a new functions-...php : it depends on how many new function there will be. It seems to me that we'll need just 4 new core functions (get timezoned timestamp, get offset, get formatted date, get formatted time) so it doesn't "deserve" a new file on its own, but you tell me if I overlook something.

Point 2 : in core we just need "simple" functions to output stuff, with filters on which plugins can hook, and that will not change current behaviors if there's no plugin. Something like

function yourls_get_formatted_date($timestamp) {
    $format = yourls_apply_filter( 'get_formatted_date_format', 'M d, Y' ); // tap into this with the plugin
    return yourls_apply_filter( 'get_formatted_date', date( $format, $timestamp ) );
}

Regarding default formatting option, the best would be to keep things unchanged for people who won't use the plugin. Seems to be 'M d, Y H:i' at the moment

@ozh
Copy link
Copy Markdown
Member

ozh commented Apr 9, 2020

@JeremyKeusters hello man, just checking in to see if everything is OK. Feel free to ask for help if needed !

@ozh
Copy link
Copy Markdown
Member

ozh commented Apr 15, 2020

@JeremyKeusters ping

@JeremyKeusters
Copy link
Copy Markdown
Contributor Author

Everything okay, yes! :) I'm sorry for not replying, I'm currently working on a small project that shouldn't take too long. After that, I'm planning on spending some time on this again!

@ozh ozh mentioned this pull request Apr 30, 2020
3 tasks
@ozh ozh mentioned this pull request May 9, 2020
@ozh
Copy link
Copy Markdown
Member

ozh commented May 9, 2020

Closing here, continuing in #2684

@ozh ozh closed this May 9, 2020
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.

named time zone support

4 participants