Parse Timestamps in Messages: Core Implementation #5253

aero31aero · 2017-06-06T11:30:31Z

Code needs cleaning up as well as decisions regarding the display format.

Example: !timestamp(Jun 6 2017 4:48 PM) should render as Tuesday, June 6th 2017, 10:18:00 pm (Timezone: UTC+05:30).

Question: What should we do with timestamps that are incorrect or not parsable?
Currently, I've done it so that no error messages are displayed and the original text is kept intact.

Since right now the syntax only picks up timestamps in UTC, I'm looking for ways to add support for timezones using dateutil.parser.

Some sample runs:

timabbott · 2017-06-06T15:40:37Z

@aero31aero this is a great start! A few thoughts, looking at this:

For errors, I think we could follow the pattern of our LaTeX support and just show what entered with some coloring to show they did it wrong? The other option is to do nothing special.
Maybe we want this to be called !time, not !timestamp? That feels more user-friendly in retrospect.
Let's start adding tests to zerver/fixtures/bugdown-data.json to encode decisions like how to handle errors, etc.
Let's scope the CSS to inside a .message_content, since I can easily see us having a .timestamp in something.
For the format, I have a few thoughts. First, I think we don't need the Timezone:. And am/pm should be capitalized (assuming the user's specified display time format isn't military time; we should be using page_params.timezone and page_params.twenty_for_hour_time). Other than that,

<span class="timestamp" value="1496767680">Tuesday, June 6th 2017, 10:18:00 PM (UTC+05:30)</span>

looks pretty reasonable, though it'd be nice if we had a good way to not include the :00 seconds in the case that they're not needed.

aero31aero · 2017-06-06T16:01:56Z

I was thinking of using colors as well, and would give that a go. For regular correctly parsed values, I was thinking of a simple grey colored or similar highlight so that it doesn't feel obtrusive to the user.
2/3/4: Okay.
About parsing timezones. If users don't have timezone set in their profile, I'd revert to using the current system's timezone. Or, in those cases showing timezone would be useful, I guess.

Making the changes now.

aero31aero · 2017-06-07T09:03:53Z

24 Hours time:

12 Hours time:

@timabbott this seems fine?
Edit: Ignore the blue highlight in first case. That's just Firefox Dev Tools.

aero31aero · 2017-06-07T19:59:20Z

Squashed all commits. If you could review this fast and probably have it merged before the next round of meetings, that could give the feedback required for making changes I suppose.
TODO: Add support for specifying time in time zones other than UTC. Would add a commit when I get it working.

timabbott · 2017-06-08T16:28:09Z

Hmm, I think we do want the timestamp elements set apart, but I'm not sure the green color is the right way to do that. @brockwhittaker @jackrzhang any ideas what we should use there?

brockwhittaker · 2017-06-12T23:56:59Z

I would get rid of the "hrs". If you're on 24-hour time, you don't need anything after. I would also change "not a good time" to "Invalid Date", since that's more standard.

The new color you've changed it to here looks good. You have a padding-right: 4px but you should also add the same padding on the left; the clock is too close to the edge. I'd also then add some padding between the text and the clock.

Otherwise looks good now!

aero31aero · 2017-06-14T00:53:53Z

I've made the changes @brockwhittaker . Also, the default is to show the text that the user entered, and not a good time was just my trial text. Sorry for the confusion. Here is the updated screenshot:

I'll try to integrate a timepicker with this UI in the composebox now..

lonerz · 2017-06-14T02:10:29Z

Seems like Case 5 still says "not a good time" :)

aero31aero · 2017-06-14T08:17:28Z

Guess I did really choose a bad sample string...
@lonerz see this one. Sorry for the confusion.

timabbott · 2017-06-14T15:56:32Z

@aero31aero it looks like you haven't updated the markdown docs? There's 2 copies, a short-form in-app version and a long-form /help/ version.

While you're at it, would you be up for updating docs/markdown.md to make clear any changes need to be documented (in its own commit)? Thanks!

timabbott · 2017-06-14T15:58:46Z

A few more thoughts:

I think we should use "AM", not "am", for consistency with what we do elsewhere in Zulip.
Maybe we should make the styling look more similar to mentions visually?

timabbott · 2017-06-14T16:00:03Z

zerver/fixtures/bugdown-data.json

+      "input": "!time(hello world)",
+      "expected_output": "<p><span class=\"timestamp\"><img alt=\":clock4:\" class=\"emoji\" src=\"/static/generated/emoji/images/emoji/unicode/1f553.png\" title=\":clock4:\"><span>hello world</span></span></p>",
+      "bugdown_matches_marked": false
+    },


why does bugdown not match marked for these? I think that's probably a sign that the frontend implementation is buggy.

I was of the opinion that if there are any differences in what can be parsed and what cannot be, that would cause ambiguity. Worst case, someone would immediately see something as having been parsed but later realize that it actually wasn't. So, we should have that logic in one place only.

Well, these are the tests for the markdown processor; at least the specific examples we wrote tests for should match, right?

(Also, we should have both a unix timestamp and full date test cases, right?)

Yes, those examples should. I was still trying to get the UI passed, though. I'll add more tests and make the marked implementation pass them as well.

timabbott · 2017-07-08T02:28:30Z

@aero31aero this one feels pretty close to completion; do you time to get it into a mergable state? I'd love to have this feature :)

aero31aero · 2020-05-08T16:59:44Z

There are minor display ideas from Rishi's comment that could be implemented, but this should be good for a trial run on CZO.

Maybe we can iterate with minor changes on top of this in a later PR.

static/js/timerender.js

timabbott · 2020-05-09T01:01:23Z

Posted comments on the timerender commit; I think with a few fixes we can merge that piece at least.

static/js/markdown.js

andersk · 2020-05-12T20:48:35Z

static/js/timerender.js

+    let timestring = time.format('ddd, MMM D');
+    if (now.year() !== time.year()) {
+        timestring += time.format(' YYYY');
+    }
+    timestring += ',';


This kind of code is going to be tricky to localize. Moment has a fairly comprehensive collection of auto-localizing formatting codes, can we just use those?

Yeah, I'd love to use something from moment rather than this big block of code if there's something appropriate.

aero31aero · 2020-05-12T21:51:52Z

Tests are passing. I wrote a small TODO list for the work left in https://chat.zulip.org/#narrow/stream/6-frontend/topic/timestamp, which is all just frontend related work and the html syntax is final.

This PR has gone on for too long, I'll work on those frontend TODOs in a new one.

timabbott · 2020-05-12T23:16:12Z

Can you move the cleanup commits that aren't part of this feature to the start of the branch, with proper commit messages? E.g. the tab_list commit doesn't explain the problem it solves or why it doesn't break something important to make that change.

aero31aero · 2020-05-13T07:09:37Z

Pushed the cleanup commits to top and added more details in the top-bar CSS change commit.

aero31aero · 2020-05-19T18:45:15Z

@timabbott do you have time to look at this?

static/js/timerender.js

This code generates the timestamp string to be shown to the user from the given timestamp in unix format using moment.js.

We leverage the composebox typeaheads to show flatpickr to pick dates and times for the !time syntax.

We use moment.js to try and parse the time from current token. If we are successful, we init flatpickr with the parsed time, else we default to using the current time.

…one.

We display stream descriptions in the tab-bar that is rendered by our markdown pipeline, thus we have support for timestamps in it. This commit adds the frontend bit where we replace the raw timestamp with a human readable version.

This commit shifts our timestamp syntax to be of the form: since value is not a valid attribute of span elements.

static/js/composebox_typeahead.js

static/styles/rendered_markdown.scss

aero31aero · 2020-05-21T08:48:33Z

Finally, closing this in favor of the 3 individual PRs that have been split from this. 🎉

zulipbot added the needs review label Jun 6, 2017

aero31aero force-pushed the timedown branch from 0dc6dab to e7edbd1 Compare June 6, 2017 11:34

aero31aero force-pushed the timedown branch 2 times, most recently from 6570ff1 to 993a1b9 Compare June 7, 2017 18:29

timabbott mentioned this pull request Jun 8, 2017

markdown: Add support for multi-timezone timestamps #5176

Closed

aero31aero force-pushed the timedown branch 3 times, most recently from 3014030 to b0481f7 Compare June 11, 2017 14:17

aero31aero force-pushed the timedown branch 2 times, most recently from 9cda4c2 to c6e434d Compare June 14, 2017 00:49

timabbott reviewed Jun 14, 2017

View reviewed changes

zulipbot added needs more work and removed needs review labels Jun 14, 2017

aero31aero force-pushed the timedown branch from c6e434d to b3469bb Compare July 8, 2017 12:25

zulipbot added needs review and removed needs more work labels Jul 8, 2017

aero31aero force-pushed the timedown branch from b3469bb to 5aad08c Compare July 8, 2017 12:47

adnrs96 mentioned this pull request Jul 9, 2017

Make code base compliant with PEP-8 E-261. #5144

Closed

timabbott reviewed May 9, 2020

View reviewed changes

static/js/timerender.js Outdated Show resolved Hide resolved

timabbott reviewed May 9, 2020

View reviewed changes

static/js/timerender.js Outdated Show resolved Hide resolved

aero31aero force-pushed the timedown branch from aab935b to 2b2e46e Compare May 12, 2020 16:19

andersk reviewed May 12, 2020

View reviewed changes

static/js/markdown.js Outdated Show resolved Hide resolved

andersk reviewed May 12, 2020

View reviewed changes

aero31aero force-pushed the timedown branch from 01b5bc6 to 93d9ebe Compare May 12, 2020 21:25

aero31aero force-pushed the timedown branch from 93d9ebe to 93a0b88 Compare May 13, 2020 06:46

timabbott force-pushed the timedown branch from 93a0b88 to 0476981 Compare May 20, 2020 21:24

timabbott reviewed May 20, 2020

View reviewed changes

static/js/timerender.js Show resolved Hide resolved

aero31aero added 6 commits May 20, 2020 14:28

markdown-timestamp: Add rendering functions for timestamps.

a4518b8

This code generates the timestamp string to be shown to the user from the given timestamp in unix format using moment.js.

markdown-timestamp: Show flatpickr using typeaheads.

f79bd2c

We leverage the composebox typeaheads to show flatpickr to pick dates and times for the !time syntax.

markdown-timestamp: Maybe autofill flatpickr when editing a timestamp.

1d36153

We use moment.js to try and parse the time from current token. If we are successful, we init flatpickr with the parsed time, else we default to using the current time.

markdown-timestamp: Use browser's time instead of user inputted timez…

45b2e18

…one.

markdown-timestamp: Use data-timestamp attribute.

fc4a927

This commit shifts our timestamp syntax to be of the form: since value is not a valid attribute of span elements.

timabbott force-pushed the timedown branch from 0476981 to fc4a927 Compare May 20, 2020 21:28

timabbott reviewed May 20, 2020

View reviewed changes

static/js/composebox_typeahead.js Show resolved Hide resolved

timabbott reviewed May 20, 2020

View reviewed changes

static/js/composebox_typeahead.js Show resolved Hide resolved

timabbott reviewed May 20, 2020

View reviewed changes

static/styles/rendered_markdown.scss Show resolved Hide resolved

This was referenced May 21, 2020

(timestamp #3) Show Flatpickr for picking timestamp. #15052

Closed

(timestamp #2) Extract rendered_markdown.js to update dynamic elements. #15049

Closed

(timestamp #4) markdown-timestamp: Document the syntax in /help pages. #15054

Closed

aero31aero closed this May 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parse Timestamps in Messages: Core Implementation #5253

Parse Timestamps in Messages: Core Implementation #5253

aero31aero commented Jun 6, 2017

timabbott commented Jun 6, 2017 •

edited

aero31aero commented Jun 6, 2017

aero31aero commented Jun 7, 2017 •

edited

aero31aero commented Jun 7, 2017

timabbott commented Jun 8, 2017

brockwhittaker commented Jun 12, 2017

aero31aero commented Jun 14, 2017

lonerz commented Jun 14, 2017

aero31aero commented Jun 14, 2017

timabbott commented Jun 14, 2017 •

edited

timabbott commented Jun 14, 2017

timabbott Jun 14, 2017 •

edited

aero31aero Jun 14, 2017

timabbott Jun 14, 2017

aero31aero Jun 14, 2017

timabbott commented Jul 8, 2017

aero31aero commented May 8, 2020

timabbott commented May 9, 2020

andersk May 12, 2020

timabbott May 12, 2020

aero31aero commented May 12, 2020

timabbott commented May 12, 2020

aero31aero commented May 13, 2020

aero31aero commented May 19, 2020

aero31aero commented May 21, 2020

Parse Timestamps in Messages: Core Implementation #5253

Parse Timestamps in Messages: Core Implementation #5253

Conversation

aero31aero commented Jun 6, 2017

timabbott commented Jun 6, 2017 • edited

aero31aero commented Jun 6, 2017

aero31aero commented Jun 7, 2017 • edited

aero31aero commented Jun 7, 2017

timabbott commented Jun 8, 2017

brockwhittaker commented Jun 12, 2017

aero31aero commented Jun 14, 2017

lonerz commented Jun 14, 2017

aero31aero commented Jun 14, 2017

timabbott commented Jun 14, 2017 • edited

timabbott commented Jun 14, 2017

timabbott Jun 14, 2017 • edited

Choose a reason for hiding this comment

aero31aero Jun 14, 2017

Choose a reason for hiding this comment

timabbott Jun 14, 2017

Choose a reason for hiding this comment

aero31aero Jun 14, 2017

Choose a reason for hiding this comment

timabbott commented Jul 8, 2017

aero31aero commented May 8, 2020

timabbott commented May 9, 2020

andersk May 12, 2020

Choose a reason for hiding this comment

timabbott May 12, 2020

Choose a reason for hiding this comment

aero31aero commented May 12, 2020

timabbott commented May 12, 2020

aero31aero commented May 13, 2020

aero31aero commented May 19, 2020

aero31aero commented May 21, 2020

timabbott commented Jun 6, 2017 •

edited

aero31aero commented Jun 7, 2017 •

edited

timabbott commented Jun 14, 2017 •

edited

timabbott Jun 14, 2017 •

edited