shout icon indicating copy to clipboard operation
shout copied to clipboard

Add show timestamp toggle

Open floogulinc opened this issue 8 years ago • 8 comments

Closes #93

floogulinc avatar Sep 27 '15 19:09 floogulinc

Nice addition @floogulinc, thanks!

However, I notice that, on mobile (the smallest version), Shout by defaults hides the timestamp. With your PR, that makes your checkbox appear non working there. I see 2 possible fixes:

  • easy: display timestamp by default on mobile
  • difficult: keep the current default on mobile, but adapt the checkbox and code accordingly

I personally prefer the first solution. Not only it's what I expect on mobile by default (plus it's consistent with other sizes and we can probably merge 2 width categories), but it's also much simpler in code. Solution 2 forces you to check the width of the screen using JS, ... urgh.

What do you think?

astorije avatar Oct 07 '15 03:10 astorije

@astorije You can remove timestamps from mobile by removing

#chat .time {
    display: none;
}

from the @media (max-width: 479px) section of the css. But by doing that, messages will take up one extra line:

I honestly don't know if there is enough room on mobile (phones specifically) to do timestamps, at least not in the same way we have them everywhere else.

floogulinc avatar Oct 07 '15 10:10 floogulinc

Urgh, 2 lines are ugly.

@floogulinc, I am not sure we can make this decision here: some mobiles have gigantic screens, some don't. And fashion goes to increase your screen size as you upgrade your phone, sadly...

My proposal still holds: let's show timestamp by default (first solution) and use your checkbox to please those who would like more room. At the moment, mobile users cannot even display the timestamp, which is very annoying. Your PR will let them (us!) do so while easily switching to no timestamp again.

Thanks! I honestly can't wait for this PR to make it live!

astorije avatar Oct 07 '15 12:10 astorije

@astorije Just note that that the screenshot is from a 6 inch 1440p phone. So it seems pretty ugly on any phone in portrait, If i rotate it then it switches to "tablet" view and works fine with the timestamps.

floogulinc avatar Oct 07 '15 14:10 floogulinc

+1 again for this. I often miss the timestamps and and have to rotate my device to see them. Kind of sucks. I thinks it would be fine even if the nicks won't line up vertically. I would love this feature!

richrd avatar Oct 07 '15 17:10 richrd

@floogulinc, I'm not sure about your meaning, are you agreeing with me? What your screenshot shows me is that it does look ugly indeed in portrait mode, and we would better off putting everything on the same line everywhere. Oh and thanks for making me discover that on landscape mode the timestamp appears! How inconsistent :-/

Actually, @richrd has a good point, we do not need to have the timestamps line up on mobile. I think the best result would be to have on mobile (00:00) username message, using :before and :after to add the parentheses. Would you be OK with that @floogulinc? That would be terrific if you could add that to your PR as it's directly related.

So in a nutshell, here is my final offer:

  • Checkbox to show the timestamp, enabled by default (done)
  • Display the timestamp on mobile by default
  • Surround timestamp on mobile with () and remove alignment between the timestamp and the nick

astorije avatar Oct 08 '15 02:10 astorije

@floogulinc, any updates on this? :-)

astorije avatar Oct 15 '15 02:10 astorije

@floogulinc, could you make the small change I suggested so that this can go live? It really is a shame we could not finalize this earlier, but I hope we can still do it :-) If improvements are needed, then fine, but at the slow pace we are going overall, each small step is meaningful.

astorije avatar Jan 06 '16 03:01 astorije