Initial custom timestamps.

[AlMcKinlay]

Initial version of custom timestamps. Couple of small issues unresolved, and I thought I would gain some feedback.

1. After changing the timestamp format, only new messages currently get the new format. Either we would have to refresh the page or reformat everything (not spent much time investigating how that would be done, but I imagine it would mean essentially clearing all messages out of the dom first)
2. The spacing is a little weird just now. With the specified width, it wouldn't work with longer timestamps (it just overlaps the name), but with the way I've done it, the spacing is different for most rooms.

With HH:mm timestamp

After changing to HH:mm:ss timestamp:

After refreshing:

[xPaw]

Regarding the non changing timestamps: .time could have a data attribute with a raw timestamp, then when the setting is changed, you can do chat.find('.time') and reformat it easily.

[AlMcKinlay]

Alright, I have reverted my css change, because it was just not ideal. I'll have a think later about a better way to do it.

I've also changed the time handler to use a variable rather than re-rendering it. It's not ideal, but it's better than the other options, I think.

Finally, I've modified the cookie to use the same cookie and the same setting/desetting code.

Still to do:

1. Sort out the css for the time
2. Find a good way to rerender times (I'll look into @xPaw 's idea)

[xPaw]

Here's an interesting thought: if the timestamp setting is empty, it shouldn't invoke moment to format the time, and .time element would be hidden.

[AlMcKinlay]

Yeah, @xPaw, not to sound like I'm just stealing your idea but I was actually thinking about that today. I'm not sure how user-friendly that is, but it would mean 1 less setting being added in the future (to turn off timestamps).

Anyone else got thoughts on that?

[xPaw]

I'm not sure how user-friendly that is, but it would mean 1 less setting being added in the future

Adding a description next to the input would help.

[astorije]

Hey @YaManicKill, I haven't reviewed the code of this yet, but I wanted to share my two cents.

To give you a few words of context, the last projects I have been involved in (mostly for work) either focused on or left out entirely usability and user experience, and that made a whole world of difference. Since then, I have been very careful about the UX of products I use or develop, and since my company recently hired a dedicated UX lead, I can't promise I won't get even more pedantic about it :D

I am really not fond of having a timestamp format input at all. This is one of those cases where customization complexifies things unnecessarily. Also, an empty input meaning no timestamp is very cryptic (even with a help description, which shouldn't necessary for this simple setting).

If you think about it, the 2 cases HH:mm and HH:mm:ss cover pretty much everything users will set. Add to that 24-hour and 24-hour formats (to not leave our American friends :-) ) and really that's about it. I don't possibly see a use case where only hours, minutes, or seconds would be relevant to more than 0.001% of our users :-) (And if really someone wants to have such a customized setup, then that's where the package system comes in handy!)

Just to see how design hipsters are doing, I opened the time settings of the Mac I'm on right now:

This is very simple and clear IMO. If we relate to that, one checkbox for displaying seconds, one checkbox (or 2 radio buttons / a select element) for the 12/24-hour format switch, and deal! :-) I prefer 2 simple, descriptive checkboxes, or even 4, to a fully customizable input field that requires a description and maybe a link to a doc (to detail HH, mm, ...) to be used.

Do these arguments make sense to you? I'm sorry to receive your PR negatively while you have been contributing a lot to this project though :-/

Internally, keeping a string for the time format makes a lot of sense though (I mean, even if we didn't need one to give to moment.js), as it offers a possibility to extend the system by manipulating it.

[astorije]

Mentioning https://github.com/erming/shout/pull/484 here for the record.

[AlMcKinlay]

Fair point.

[AlMcKinlay]

Ok, to clean up, I'll remove this one, and we can look at just redoing #578 and #484 (with possibly a second one for mobile).