Wikipedia:Code review/UTCLiveClock

The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.

UTCLiveClock edit

Code review of potential changes to UTCLiveClock gadget
Review requested by DannyS712 at 10:23, 11 May 2020 (UTC)[reply]

Current code: mw:MediaWiki:Gadget-UTCLiveClock.js

Proposed new code: mw:Special:Permalink/3845287

Changes to code: mw:Special:Diff/3845287

Summary of changes:

Add support for timezones other than UTC by allowing the user to specify `window.LiveClockOffset` in their js (hours relative to UTC). No change in functionality if not set

Specific issues / areas for feedback:

Is this something the gadget (on mediawiki, but widely imported, including in enwiki's local version, see MediaWiki:Gadget-UTCLiveClock.js) should include?

Thanks, DannyS712

Discussion

Posted notes at WP:VPT and WP:IAN --DannyS712 (talk) 10:38, 11 May 2020 (UTC)[reply]

I personally would like to see this feature on the enwiki. I have facepalmed myself for using it as a clock instead of looking at my computer for the current time and I am not good at math. Galendalia _{CVU Member} \ ^{Chat Me Up} 11:24, 11 May 2020 (UTC)[reply]

@DannyS712: to be clear, you want to fork this to be a local gadget right? — xaosflux ^Talk 13:41, 11 May 2020 (UTC)[reply]
@Xaosflux: no; I intend to make this change to the version of the gadget on mediawiki.org, and wanted to run it by the enwiki community since the the local gadget merely imports the version on mediawiki.org --DannyS712 (talk) 14:14, 11 May 2020 (UTC)[reply]
@DannyS712: I've got to imagine that other projects may use that too - also doesn't seem like it is updated often, I'd be fine just forking it locally and eliminating another cross-domain dependency. — xaosflux ^Talk 14:21, 11 May 2020 (UTC)[reply]
Yes, other projects use this too, but enwiki is the largest; I have no issues with enwiki forking it, though this then becomes a discussion of should the edit be made to the local version rather than the mediawiki.org version --DannyS712 (talk) 14:25, 11 May 2020 (UTC)[reply]

Forking doesn't seem necessary. (As for cross-domain, I assume that's a concern for ownership rather than concern about the Javascript attack vector.) --Izno (talk) 14:35, 11 May 2020 (UTC)[reply]
Will people have to manually change their offset every time DST rolls around? – Jonesey95 (talk) 15:18, 11 May 2020 (UTC)[reply]
Yes - the offset is specified in number of hours relative to UTC --DannyS712 (talk) 15:32, 11 May 2020 (UTC)[reply]
If you (for reasons unclear to me) want your local timezone, you could set the option to new Date().getTimezoneOffset() / 60 in your personal JS so it'll stay updated. I'm not sure what happens for timezones that are a half-hour apart. Probably worth testing that scenario. — MusikAnimal ^talk 16:57, 11 May 2020 (UTC)[reply]
Having to manually change a computer-based clock for DST in 2020 is a show-stopper. DannyS712, please consider using timezone as a setting instead of a straight UTC offset. – Jonesey95 (talk) 18:02, 11 May 2020 (UTC)[reply]
Thanks for developing this! $ɱ$ (talk) 15:58, 11 May 2020 (UTC)[reply]
I have an alternative version that I was working on independently: User:Writ_Keeper/Scripts/timezone.js. It works similarly, except that the user specifies a particular time zone (or "local", if they just want their local time zone). Time zone names can be found at List_of_tz_database_time_zones, which we should probably link to in the gadget description (or, rather, probably the official document that that is presumably sourced from). I feel like that's a better implementation, because asking people to modify their UTC offset every time daylight savings time rolls around is kind of a tall order. That said, code-wise, Danny's code looks mostly okay, and I don't think there's much wrong with it. The one thing I would point out is that I believe that there are some time zones (in India, I think?) that are a half-hour off of UTC, not just an hour off, so Danny's implementation doesn't perfectly capture all time zones. Writ Keeper ⚇♔ 16:02, 11 May 2020 (UTC)[reply]
- Diff for my version Writ Keeper ⚇♔ 16:05, 11 May 2020 (UTC)[reply]
Seems fine, though I would call the option UTCLiveClockOffset to match the name of the gadget, and that it is in fact the offset from UTC. I have not tested the code. — MusikAnimal ^talk 16:57, 11 May 2020 (UTC)[reply]

Discussion part 2

@MusikAnimal, Writ Keeper, and Jonesey95: I rewrote my update. Net changes: mw:Special:Diff/3845284/3880447 - new code: mw:Special:Permalink/3880447. This uses timezone names, the option is LiveClockTimeZone, and it should capture the issues discussed above. Thoughts? Thanks, --DannyS712 (talk) 12:33, 28 May 2020 (UTC)[reply]

I'm definitely looking forwards to this, if it's not too much trouble please ping me when it's finished. Doug Weller talk 08:15, 7 June 2020 (UTC)[reply]

Thanks for incorporating my suggestions. I have no further comments. I haven't tested it. – Jonesey95 (talk) 14:02, 7 June 2020 (UTC)[reply]

Last call before I merge the changes - any objections? --DannyS712 (talk) 20:05, 30 June 2020 (UTC)[reply]

I've merged the changes --DannyS712 (talk) 17:48, 20 July 2020 (UTC)[reply]

The discussion above is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.