Add timeline app#410
Conversation
aJanuary
force-pushed
the
add-timeline
branch
3 times, most recently
from
106096c to
70a8273
Compare
|
This is low stakes. I won't mind if this gets rejected out of hand or languishes on the review pile for a while. |
offbyone
left a comment
offbyone
left a comment
There was a problem hiding this comment.
Some comments that I'd want addressed before merging it.
Also, please add it somewhere in the docs.
(yes, the docs are a mess. I want to clean those up, but just have not yet got around to thinking through an organization. Put it somewhere in the admin section, as its own file for now)
| # Convention admin utilities and seeding commands | ||
| "nomnom.convention_admin", | ||
| # Admin-configurable homepage timeline | ||
| "nomnom.timeline.apps.TimelineConfig", |
There was a problem hiding this comment.
This is a bit weird naming-wise; why is it not just nomnom.timeline?
| {% load i18n waffle_tags %} | ||
| {% block content %} | ||
| <div class="container"> | ||
| {% include "timeline/timeline.html" %} |
There was a problem hiding this comment.
Look at other waffle tags; this should be enabled with a switch. There is an example of creating one in src/nomnom/canonicalize/migrations/0006_create_sankey_switch.py and the use of {% switch "advisory_votes" %} elsewhere in this file.
|
|
||
|
|
||
| def index(request: HttpRequest) -> HttpResponse: | ||
| timeline_events = list(TimelineEvent.objects.all()) |
There was a problem hiding this comment.
The better way to add this functionality is via the index_content_load signal; that way the base app doesn't have direct dependencies on the other apps. You can also make that load conditional on the switch too.
There was a problem hiding this comment.
Currently index_content_load only triggers for authenticated users. Are we happy with the timeline only being visible for authenticated users?
There was a problem hiding this comment.
That feels like a bug. I'm actually surprised.
There was a problem hiding this comment.
Aesthetically I don't love the way it works on mobile when there are more timeline events. Would it make sense to hide it behind an expanding details in some way, and be a vertical timeline when on mobile?
There was a problem hiding this comment.
I'm thinking of something like this where it shows the current and next event (specifically, the first transition from a completed to a non-completed event). Clicking/tapping it will open the full vertical timeline. Obviously needs a lot of tweaking to the spacing and gradients.
There was a problem hiding this comment.
There's the caveat that doing this sort of thing complicates the already slightly complicated CsS, especially doing it in an accessible way. So there's the question of whether you're okay taking on that extra maintenance burden.
There was a problem hiding this comment.
Yeah, my CSS-fu is, uh, not great. If you think you can make this relatively idiot-resistant (not idiot-proof, since that's how you evolve better idiots :) ) then I'd be okay with it anyway.
offbyone
added
the
minor
This allows admins to configure a timeline that shows on the homepage. Events are rendered as bubbles with a label and the date shown beneath them. If the event is marked as complete, then the bubble is filled. The date is a free-text field, allowing both specific dates (e.g. "Feb 12th 2016") and vague dates (e.g. "Early May 2026"). Because what events should be shown can be specific to a particular year and what that year's Hugo admin wants to communicate, there is no automation tied to the timeline. It is up to the admin to remember to update vague dates with more specific ones, and mark events as completed once they are done. If no timeline events are configured, the timeline doesn't show, allowing the Hugo admin to opt in to whether they have the timeline or not.
Rather than allowing the timeline to wrap, on small screens switch to a vertical view. In order to avoid taking up all the screen real estate in the vertical view, default to rendering it collapsed so only the "current" time is shown. This is the first transition from a completed to a non-completed event. The timeline can then be interacted with to toggle the non-collapsed view.
|
I keep coming back to the fact that this makes more sense as a convention website item, not something data-driven. I think this should be in those places instead. I think, thanks for the work, but I'm not going to add this feature. |
2 participants
This allows admins to configure a timeline that shows on the homepage. Events are rendered as bubbles with a label and the date shown beneath them. If the event is marked as complete, then the bubble is filled.
The date is a free-text field, allowing both specific dates (e.g. "Feb 12th 2016") and vague dates (e.g. "Early May 2026").
Because what events should be shown can be specific to a particular year and what that year's Hugo admin wants to communicate, there is no automation tied to the timeline. It is up to the admin to remember to update vague dates with more specific ones, and mark events as completed once they are done.
If no timeline events are configured, the timeline doesn't show, allowing the Hugo admin to opt in to whether they have the timeline or not.