CSS tidyup, Table glitch fix, sticky "Newer version" banner #169
Conversation
osfameron
requested review from
leeming,
dgnicholson,
simon-dew and
sarahlwelton
| .mb-lg-5 { | ||
| margin-bottom: var(--base-large-space) !important; /* 32px */ | ||
| } | ||
| } |
There was a problem hiding this comment.
Just curious why we deleted all of this spacing CSS?
There was a problem hiding this comment.
I couldn't find any evidence of it being used.
It looks like tailwindCSS convention, which isn't used elsewhere in the project, so I think it's an experiment that wasn't furthered.
| /* Use this for a working pseudo element before an [abstract] attribute-marked block in the documentation. | ||
| This one adds the words "Quick Summary" before any text inside the [abstract] block. */ | ||
|
|
||
| /* | ||
| .doc .abstract blockquote::before { | ||
| content: "Quick Summary \a"; | ||
| white-space: pre; | ||
| color: var(--color-muted); | ||
| font-weight: var(--weight-medium); | ||
| font-size: var(--heading-h4); | ||
| padding-bottom: 1rem; | ||
| } */ | ||
|
|
There was a problem hiding this comment.
You asked me to leave this in, in case we wanted to use it as a working example of a Pseudo :before in the future :) Sure you want to delete it?
There was a problem hiding this comment.
Hahaha, good catch.
(Trying to purge commented CSS as it adds to cognitive weight, but will have a think...)
| @@ -208,7 +200,7 @@ | |||
| } | |||
|
|
|||
| .navbar-dropdown li.current a.navbar-item::before { | |||
| content: "\2023"; | |||
| content: "\2023"; /* ‣ */ | |||
There was a problem hiding this comment.
Is it okay to have a special character like that in here?
There was a problem hiding this comment.
pretty sure, inside a comment.
(I suspect it'd be fine inside the quotes too, but haven't tested extensively...)
| @@ -129,3 +117,57 @@ | |||
| .toc ul li[data-level="2"] a { | |||
| padding-top: 2px; | |||
| } | |||
|
|
|||
| /* Sidebar Box and Tools */ | |||
There was a problem hiding this comment.
Is this for the Redocly integration, or?
There was a problem hiding this comment.
Nope, this PR is intended to be a refactor, so it's just decribing the existing sidebar box.
src/css/table.css
Outdated
There was a problem hiding this comment.
I guess you're just tidying up at the moment, but this file still contains the problematic table caption styles?
There was a problem hiding this comment.
That's right, this PR is a refactor and shouldn't contain any new features (and with a bit of luck, no new breakage either... 😅)
There was a problem hiding this comment.
(NB: now not true, see the most recent commits)
It's nice to credit contributors... but doing so in an invisible div that makes an API request to Github on every page load perhaps not.
Refactor callouts.css base.css - document, purge tailwind css-tidyup Comments. Merge toc.css and toolbar.css css-tidyup minor components
Glitch reported by @malarky. Removed special display/position handling.
Make article-banner ("A newer version of this document is available")
sticky.
This requires updating the scroll-top, otherwise when clicking on a link
it would be hidden behind the banner.
The existing handling already accounts for the same issue, but with the
height (7.1rem) of the top nav.
We use a dynamic css `var(--scroll-margin-top-space)` which can be
overridden in the case that we have a banner.
osfameron
changed the title
| @@ -5,7 +5,6 @@ | |||
| {{> labels}} | |||
| </div> | |||
| {{/if}} | |||
| {{> contributor-bot}} | |||
There was a problem hiding this comment.
What was contributor-bot? Why are we deleting it?
There was a problem hiding this comment.
Go to e.g. view-source:https://docs.couchbase.com/server/current/getting-started/do-a-quick-install.html and you'll see <div class="contributor-list-box">
Inspect the DOM and you'll see it's been filled with
<ul id="contributorList"><li><a href="https://github.com/mojavelinux" target="_blank"><img src="https://avatars.githubusercontent.com/u/79351?v=4" alt=""></a></li><li><a href="https://github.com/cb-docs-robot" target="_blank"><img src="https://avatars.githubusercontent.com/u/25054062?v=4" alt=""></a></li><li><a href="https://github.com/anmol-algo" target="_blank"><img src="https://avatars.githubusercontent.com/u/58175473?v=4" alt=""></a></li><li><a href="https://github.com/RayOffiah" target="_blank"><img src="https://avatars.githubusercontent.com/u/77050471?v=4" alt=""></a></li><li><a href="https://github.com/amarantha-k" target="_blank"><img src="https://avatars.githubusercontent.com/u/13425257?v=4" alt=""></a></li></ul>
e.g. it's doing an API request to github stats for docs-ui on every page load to fill this in.
|
(Left too long unmerged, will have to come back to this to see if there's anything that can be introduced -- as smaller PRs...) |
Some reorg work I did while trying to get head around our CSS.
As I want to use this as basis for future work, this branch does now also include 2 features:
NB: this second fix does use dynamic css variables, which means:
(I think it's worth pushing in any case, and fixing issues as we come across them, but happy to hear concerns, or for 👀 to point out any concrete issues with the implementation!)