-
Notifications
You must be signed in to change notification settings - Fork 309
Add date separators #469
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add date separators #469
Conversation
I've pushed a revision that adjusts the vertical alignment between the text and the divider lines. I also edited the PR description with another design question, one I'd forgotten about when I first posted the PR, about the font size for the dates compared to the dates in recipient headers. |
I un-marked this as draft too hastily before re-reading what I'd said above about why it's a draft 🙂. I'll do those cleanups and tests now. |
OK, now actually ready. There are still a couple of design questions open (mentioned above in the PR description), but they're pretty subtle — so I think the best thing is to go ahead and merge without worrying about those, and we can always adjust later if @terpimost has comments once he's back. |
We'll shortly want to start doing different things for these two cases.
Thanks, LGTM! Merged. |
I removed the TODO(design), because now this element's color matches the corresponding element in the web app, and that seems like a fine way to settle the question. I didn't find a date-separator example in the Figma (and Greg didn't either, when implementing them in zulip#469.)
…arator In zulip#469 implementing date separators, Greg highlighted that the date separator's deviation from web wasn't settled yet: zulip#469 (comment) zulip#469 (comment)
I removed the TODO(design), because now this element's color matches the corresponding element in the web app, and that seems like a fine way to settle the question. I didn't find a date-separator example in the Figma (and Greg didn't either, when implementing them in zulip#469.)
…arator In zulip#469 implementing date separators, Greg highlighted that the date separator's deviation from web wasn't settled yet: zulip#469 (comment) zulip#469 (comment)
I removed the TODO(design), because now this element's color matches the corresponding element in the web app, and that seems like a fine way to settle the question. I didn't find a date-separator example in the Figma (and Greg didn't either, when implementing them in zulip#469.)
Fixes: #173
This is a draft because it needs tests and also some cleanups in the code. But from a user perspective it already works great — so I'm sharing this in order to get feedback on the visual design. As mentioned at #173 (comment) , I don't see an existing design for this in Figma, so for a first draft I've taken what's in web and adapted it.
Design questions
hsl(0deg 0% 15% / 75%)
, which is the same as the dates in the recipient headers. Here, our dates in recipient headers are lighter than that: they're black with 40% alpha. So I used that color for these, though it makes the dates lighter than in web. @terpimost do you think this is the color we want?calc(12em / 14)
and acalc(15em / 14)
applying multiplicatively. (This bug illustrates why in the CSS for zulip-mobile's webview, we userem
and notem
.) So I'm not sure that difference is intended, and in this draft I've made the size the same as in the recipient headers.The vertical alignment here is just that everything's centered. But that seems to get the dividers a bit too high compared to the text. I'll have to fiddle with this (perhaps by adjusting the text's line-height).I fixed this by adding some bottom padding to the text.Other adjustments from web
Screenshot
Here's a screenshot of the current revision: