Skip to content

Restore indicator of and link to current row on agenda#3

Merged
jennifer-richards merged 16 commits into
jennifer/current-agenda-sessionfrom
jennifer/tix3697
Apr 7, 2022
Merged

Restore indicator of and link to current row on agenda#3
jennifer-richards merged 16 commits into
jennifer/current-agenda-sessionfrom
jennifer/tix3697

Conversation

@jennifer-richards
Copy link
Copy Markdown
Collaborator

@jennifer-richards jennifer-richards commented Mar 23, 2022

This restores functionality that was lost, as noted in ietf-tools#3697. Specifically, it adds a line on the agenda to indicate a reasonable approximation of "now" (though instead of red, it is now in the bootstrap theme "info" color - teal by default). It also adds a link to the page navigation panel to jump to the current spot on the agenda.

The first few commits were refactoring the javascript and should not have affected functionality.

The trickiest piece was integrating the custom content with the now automatically generated navigation panel. I've added a rudimentary mechanism for adding arbitrary content to that panel. It's likely this will need some further tweaks in addressing ietf-tools#3737.

@larseggert
Copy link
Copy Markdown

The right-hand nav panel auto-scrolls with the main content - does that not defeat the purpose here? Or are we adding a non-scrolling element somehow? (Sorry, can't test this myself at the moment.)

Comment thread ietf/static/css/ietf.scss Outdated
.ongoing>td:last-child {
background-color: red !important;
}
#agenda-table tbody tr.current-session { border-top: 0.2rem solid map.get($theme-colors, "info"); }
Copy link
Copy Markdown

@larseggert larseggert Mar 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, use bs5 variables for border-top so we can style this uniformly later?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I'll give that a try.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to work well - map($border-widths, 2) is a close match to what I eyeballed.

Comment on lines +126 to +141
if (window.meeting_timezone !== "") {
out += '<tr><th class="timehead">Meeting timezone</th><td class="text-nowrap">' +
format_time(start, window.meeting_timezone, 0) + '</td><td class="text-nowrap">' +
format_time(end, window.meeting_timezone, 0) + '</td></tr>';
}
out += '<tr><th class="timehead">Local timezone</th><td class="text-nowrap">' +
format_time(start, local_timezone, 0) + '</td><td class="text-nowrap">' +
format_time(end, local_timezone, 0) + '</td></tr>';
if (current_timezone !== 'UTC') {
out += '<tr><th class="timehead">Selected Timezone</th><td class="text-nowrap">' +
format_time(start, current_timezone, 0) + '</td><td class="text-nowrap">' +
format_time(end, current_timezone, 0) + '</td></tr>';
}
out += '<tr><th class="timehead">UTC</th><td class="text-nowrap">' +
format_time(start, 'UTC', 0) + '</td><td class="text-nowrap">' +
format_time(end, 'UTC', 0) + '</td></tr>';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will create a merge conflict with ietf-tools#3662, which also changes this. Can deal with that with a rebase though.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Hopefully it won't be too painful as most of this is just an additional level of indentation.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged ietf-tools#3662 upstream and then into this branch. I believe it went smoothly.

@jennifer-richards
Copy link
Copy Markdown
Collaborator Author

The right-hand nav panel auto-scrolls with the main content - does that not defeat the purpose here? Or are we adding a non-scrolling element somehow? (Sorry, can't test this myself at the moment.)

This adds a link to that panel that always points to "now" in the main content. I hadn't paid close attention to the auto scrolling, but I believe it still acts as before. I.e., if you click "Current session" on Tuesday, then the Tuesday link will be highlighted in the nav. That seems like the right behavior to me, even if it's an accident that gave it to us.

I think it may be necessary to add a second section to the right side panel for the time zone indicator / quick select (ietf-tools#3737). Those are clearly not navigation items, so don't belong in the existing <nav> containers. I can separate the "Current session" link from the autogenerated stuff as well. I'd toyed with that a bit, but fell back to this because it was the simplest.

* Use boostrap "card" class to format righthand panel
* Tweak container styles to better handle multiple elements
* Remove fixed height on righthand-nav item
@jennifer-richards
Copy link
Copy Markdown
Collaborator Author

@larseggert, I've reworked the panel a bit to separate out the "extra" content from the auto-generated nav links. If you have a chance to look it over I would appreciate it - it's all a bit outside the examples from the bs5 docs but seems to work.

So you don't have to try to test, here it is on the agenda page:
Screenshot from 2022-03-24 12-05-14

And here on another page where there is no extra content for the nav panel:
Screenshot from 2022-03-24 12-06-52

Comment thread ietf/static/js/agenda_timezone.js Outdated
@jennifer-richards jennifer-richards merged commit 3244e0c into jennifer/current-agenda-session Apr 7, 2022
@jennifer-richards jennifer-richards deleted the jennifer/tix3697 branch April 7, 2022 13:19
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants