Date: Add timezone hint #23400
Date: Add timezone hint #23400
Conversation
obenland
added
[Type] Enhancement
|
Size Change: +362 B (0%) Total Size: 1.14 MB
ℹ️ View Unchanged
|
|
Getting some issues when testing with some timezones:
I think it produces the
I'd expect to see |
|
Yes, this PR relies pretty heavily on the data that is bundled with Moment. For your timezone it doesn't seem to contain proper timezone abbreviations. I've not come across the "Moment Timezone has no data" error before. Does that happen for all timezones you tested with? |
It happens for all timezones. Maybe because I'm testing with WP 5.3?
|
|
Thanks for the PR! Before:
After:
Don't mind the visual changes, a good rebase should take care of that. Nice, this does seem to show the correct UTC for me. I noticed that the entire window closes as soon as you click AM/PM or a date. That's not a regression of this PR and exists in the master branch, but seems like a regression nonetheless, right?
Functionality wise, and UI wise, this looks fine enough. Though we probably need a way for a keyboard user to learn this information. It looks like it's enough to simply add |
|
I'm not sure what role would be appropriate for this piece of information. I'm also not sure if tabbing to it is the right approach, given that the user can't really interact with this content, it's purely informational. Let me do some research and ask around and I will get back to you ASAP. |
enriquesanchez
added
the
Needs Accessibility Feedback
Looks like I was wrong here. I asked for some feedback from the Accessibility team, and looks like this element can be focusable, as Joen suggested, because the tooltip is the interaction.
Thanks to @joedolson for the feedback and help. |
enriquesanchez
removed
the
Needs Accessibility Feedback
|
After some conversation on Slack and after reading the related issue, I'm not fully sure this feature should be in core. Quoting form the issue:
This doesn't appear to be a need for the vast majority of wp.org users. It is a need for a very specific group of users. This fact alone sounds like it should be plugin territory. One of the points on the related issue #15673 mentions this as well:
Given the above, and given the accessibility concerns discussed on Slack, I'd highly recommend to ship this feature in a plugin so that it can be used where it's really relevant: multi users blogs. |
|
Thank you for that feedback—I agree, it's not super useful for users that are in the same timezone as the site they're working on. I updated the PR to only display timezone information when the user timezone differs from the site timezone. That way it only gets displayed for users where that information is relevant. |
|
As someone who regularly schedules posts on both multi-user and single-user sites and who gets frustrated not knowing when the post is really scheduled for, seeing this PR land would make my life much easier when it comes to scheduling content. I have single-user sites that I've set up over time and I've moved across multiple timezones, so I always forget what timezone I may have set a site up on. And contrary to the latest commit on this PR, even if my local timezone matches that of the site I'd still want to see what timezone is being used as a comforting reminder that the time I'm scheduling in fact does match my local timezone. IOU a round of tacos and preferred paired drink for getting this into core. |
|
@ajitbohra @chrisvanpatten @youknowriad Do you have any advice on next steps for this PR? is there anyone I should ping for review? |
|
Is there anything I can do to encourage a review of this PR? |
|
I rebased this PR and tried to test it locally but for some reason the TZ indicator doesn't show up:
I did set WP timezone to be different from system. What am I doing wrong? |
draganescu
force-pushed
the
add/timezone-tooltip
branch
from
6de6e3e to
ac64607
Compare
|
@draganescu When you select one of the UTC+/-X options in the timezone dropdown it currently doesn't display the timezone info since there is no additional info to populate the popup. |
|
I'm now running into the same error as @retrofox earlier. Is there anything I can do to make sure the timezone information is bundled and available? @mkaz Do you know by chance? |
|
@obenland any chance that's affected by core bumping moment.js? https://core.trac.wordpress.org/ticket/50408 |
|
I'm looking now. There is a second npm package called moment-timezone that I believe is also required, I'm not sure how it all gets bundles and included. |
|
ok, I can commit this to the branch, but don't want to step on any toes, One thing lost is the zoneAbbr that is nice, mapping UTC-7 to PDT, but I think we can get that a different way without requiring moment. I think we could add that zone abbreviation to -- redacted -- |
|
Ok, after taking a deeper look and going to the source of the system timezone, it is easier to export the timezone abbreviation from PHP, than trying to recreate using Moment. I committed this change 9ed1320 that updates the date settings ouput via inline-script to include a new This removes the need to include moment for this component. It works if the string is set to a city/locale, for example I believe this does what we want. Please test it out by changing your WordPress timezone setting, if it differs from your user timezone (as defined by your browser) than it will show the tool tip. Thanks @obenland for continuing the effort here and the ping, this has been a long-standing issue I've wanted to resolve. A bummer we missed WP 5.5, I'm sorry I didn't see it earlier, but at least it'll finally get closed. Let me know what y'all think, it feels like a better approach. |
The system timezone already comes from PHP site, specifically from wp.date.setSettings call in wp-includes/script-loader.php This uses the get_option() to grab the string and setting and surfaces them to the wp-date package. This commit adds an additional property called `abbr` which is the short timezone such as EDT or PST, this is calculated using the PHP date and time functions.
mkaz
force-pushed
the
add/timezone-tooltip
branch
from
3520a0c to
d973b15
Compare
Converts `-03` to `UTC-3` for timzones like Argentina/Buenos_Aires. Also updates innline comments.
|
Nice work @mkaz! This works nicely for me. Feels much better without Moment. Do the changes in |
Yes, but I think that will only done at merge, so wouldn't be until WP 5.6, I can create the ticket for it in Trac. I wanted to wait until we decided this was a good approach or not. |
There was a problem hiding this comment.
It feels a little off approving, since I contributed a chunk of the code, but considering @obenland reviewed my piece and I his, it feels like it should be legit.
We can see if anyone else has any thoughts and merge tomorrow if nothing comes up.
|
Thank you Andrei 💯 |
|
Associated Trac ticket for core: https://core.trac.wordpress.org/ticket/50624 |
|
Thanks @mkaz for adding this enhancement, but I feel it's still really confusing. Here's a screenshot from a blog that has it's timezone set to PST and the user scheduling the post is in EST. The user is scheduling the post for 5am PST, which is 8am EST in her own timezone. The UI shows 5am and 8am together, which is confusing.
I'd like to suggest that we simplify this by not showing the user's local time at all. This will avoid confusion of when the post will be published, and negates the need for adding the timezone hint in the first place. Thank you for considering this feedback and suggestion. |
|
I agree its still confusing. This change is not yet available in WordPress core, it is slated for WP 5.6. So, what you are showing does not have what this PR is trying to fix, what we hope will make it less confusing. The interface should be only showing the server time and a hint at what that time is, I don't see that hint in your screenshot, for you it should be showing PST (though really it would be PDT because its daylight saving time right now) However, in testing, it does look like there is a bug and mine is showing my current time but with the label of the server which is incorrect. My local setup is opposite, I'm in PDT and the site is set to EDT.
I created an issue to address the bug in #25256 |
Description
This will add timezone information to the schedule post UI for users whose local timezone differs from the timezone set for the site. It shows the timezone abbreviation and a tooltip on hover with the full timezone name and UTC offset.
Fixes #15673.
How has this been tested?
Tested locally with various timezones set in wp-admin.
To test:
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: