Fix legislation draft version TOC width #4255
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We were using different divs to show the same text in different positions, but we can use the same one and rotate it when appropriate.
javierm
force-pushed
the
legislation_toc
branch
3 times, most recently
from
674ca3a
to
7159864
Compare
We were using JavaScript to show/hide the Table of Contents. In my humble opinion, the <details> tag has a few shortcomings [1][2], which means we should be careful about when to use it. IMHO a Table of Contents is a good candidate for this tag because it's a very common pattern to add a show/hide behavior for it, even if using it means the "navigation" role (which we are *not* using anyway) wouldn't be identified correctly. I'm adding a <details> tag to the comments section as well for consistency and in order to simplify the code. I'm not sure this is as good an application of the <details> tag, though, but then again I'm not sure about the interface we use to show/hide the comments (and this feeling is increased by the fact that we use a different interface on small screens). If we decide to change the interface in the future, we might consider using the <details> tag for the Table of Contents but not for the comments. Since the <details> tag is not supported on Internet Explorer, I'm only adding styles to this tag using the `:not([open])` option. On Internet Explorer <details> will always be opened and so these styles will be ignored. [1] https://adrianroselli.com/2019/04/details-summary-are-not-insert-control-here.html [2] https://daverupert.com/2019/12/why-details-is-not-an-accordion/
Now that comments and TOC can be closed at the same time, we use a flex layout so the main content uses the available width. We're also making the comments work better on medium-sized screens, since previously they had a fixed width and now the width is adapted to the size of the screen. Since now the comment box element has a relative position instead of an absolute one, we need to consider the draft panel height when calculating the comment box position.
Originally we were using Foundation's sticky, which wasn't entirely compatible with our way to open/close the Table of Contents because its width would not automatically be updated when the TOC was opened/closed but when users scrolled the page. Using CSS, which is now supported in most browsers, simplifies the matter. On browsers like Internet Explorer, where it's not supported, the content will not stick but other than that it'll work fine. We're also adding `scroll: auto` so when the TOC's height will be large than the page, it'll be possible to scroll it, which users couldn't do in the original version.
javierm
force-pushed
the
legislation_toc
branch
from
7159864
to
0961cf4
Compare
Senen
approved these changes
Nov 17, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Objectives
Visual Changes
Before these changes
Table of contents:
On medium screens:
On small screens:
After these changes
TODO