Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

#268 : show seconds in interface #473

Closed
wants to merge 1 commit into from
Closed

#268 : show seconds in interface #473

wants to merge 1 commit into from

Conversation

ruliane
Copy link

@ruliane ruliane commented Sep 23, 2015

Time format modified : seconds are displayed

Time format modified : seconds are displayed
@astorije
Copy link
Collaborator

I have mixed feelings about this. On one hand, I am happy that the client doesn't show second or else I (and my copy-paste) would have cognitive overflow. On the other hand, I understand some people want/need seconds to show.

I'd much prefer the original proposal: possibility to choose user-wide settings for this. Better would even be a checkbox saying "Display seconds" rather than specifying the format, much more accessible to non-tech savvy folks. Plus it's safer as there is no potential injection to prevent from.

@ruliane, would you mind adding such configuration in the User Settings?

@floogulinc
Copy link
Collaborator

@astorije I was thinking about doing that for Shuo. I can work on adding it to the settings, I have some experience there from adding theme support.

@astorije
Copy link
Collaborator

Nice @floogulinc! (Note that I added a similar comment on the related issue)

I'd be grateful if you did it here in the parent project, hoping that @erming is going to open his baby to the community a bit...

@JocelynDelalande
Copy link
Collaborator

I'd much prefer the original proposal: possibility to choose user-wide settings for this. Better would even be a checkbox saying "Display seconds" rather than specifying the format, much more accessible to non-tech savvy folks. Plus it's safer as there is no potential injection to prevent from.

+1 for this proposal

@astorije
Copy link
Collaborator

Which one, a text field with "HH:mm" in it or a checkbox to display seconds? (and a later one to support 12/24h format if that's a desired feature, etc.)

I'd definitely go for the latter!

@floogulinc
Copy link
Collaborator

@astorije The latter is what I plan on doing.

@ruliane
Copy link
Author

ruliane commented Sep 23, 2015

A "Display seconds" checkbox would probably be the best, but to answer @astorije 's question: I have no idea how to implementing such a thing. :( I simply add the patch suggested in issue #268.

@JocelynDelalande
Copy link
Collaborator

@ruliane you mean that you have no js dev skills ?

@ruliane
Copy link
Author

ruliane commented Sep 23, 2015

That's it. (I'm not a dev, js nor anything else. ;) )

@JocelynDelalande
Copy link
Collaborator

@ruliane ok (never too late to learn ;-))

So, sadly, I suggest closing this PR, as the solution is more a personal hack than a nicely packaged feature.

… Till someone comes with another PR with the checkbox thing :-)

@floogulinc
Copy link
Collaborator

@JocelynDelalande That's what im working on right now 😄

@astorije
Copy link
Collaborator

@floogulinc Good, we all agree then. Let me know if you need any help or if you have any questions, here or on the #shout-irc channel where I have installed a comfy tent.

@ruliane, it's OK, it's already very nice of you to offer your contribution. Whether we accept it, comment on it, or close it for justified reasons, we (the community) always appreciate that. Keep going and feel free to ask questions on the IRC channel!

@JocelynDelalande, I agree, and that future PR should also link here. At the moment only @erming and @ruliane can close this PR, @ruliane do you mind closing it? I don't think we'll hear from @erming right away :-)

@ruliane
Copy link
Author

ruliane commented Sep 23, 2015

Looking forward to a better PR. :)

@ruliane ruliane closed this Sep 23, 2015
@bews bews mentioned this pull request May 12, 2016
@ruliane ruliane deleted the patch-1 branch September 17, 2019 21:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants