Add collapsible support #26
Conversation
|
@tommilligan do you have any insight? |
This is going to need some thinking through... The link is there in order to generate the Copy Link Address context menu choice to copy the URL. Without it, it is hard to get a link to a particular part of the document. However, you're also correct that the expected behavior upon clicking is to toggle the collapsing. Maybe a custom |
|
Sorry for the slow reply, been a busy couple of weeks for me. The collapsible support looks really good, thanks for taking a stab at it! I think I will move anchor link generation to an appear-on-hover icon, like a lot of sites (including doc.rs for code items) do:
I think this is more discoverable anyway, and then leaves the header bar free to be clicked on to expand the dropdown/the title to contain links, etc. This should allow this PR to be merged as is, after rebase and unit tests added. |
|
Thank you a lot! However, I'm not confident enough in my git-fu to properly fix the conflict between main and this fork, without screwing the git history or opening unwanted PR or whatever. The conflict looks really easy to solve though, so I'm sure we're not to far from merging. What would you like me to do from there? |
|
No worries, thanks for you work so far. I'll work through the conflict now and let you know what needed fixing. Then I'll add a review with any outstanding tasks. |
|
For future reference, the conflict was reasonably simple to fix. You could run the following locally to reproduce what I did:
Cow::Owned(format!(
<<<<<<< HEAD
r##"<{title_block} class="admonition-title">
{title}
<a class="admonition-anchor-link" href="#{ANCHOR_ID_PREFIX}-{anchor_id}"></a>
</{title_block}>
=======
<a class="admonition-anchor-link" href="#{ANCHOR_ID_PREFIX}-{anchor_id}">
{title}
</a>
>>>>>>> Add collapsible support
"##so we can resolve as Cow::Owned(format!(
r##"<{title_block} class="admonition-title">
{title}
<a class="admonition-anchor-link" href="#{ANCHOR_ID_PREFIX}-{anchor_id}"></a>
</{title_block}>
"##
|
src/config/mod.rs
Outdated
| @@ -9,6 +9,7 @@ pub(crate) struct AdmonitionInfoRaw { | |||
| directive: String, | |||
| title: Option<String>, | |||
| additional_classnames: Vec<String>, | |||
| is_collapsible: bool, | |||
There was a problem hiding this comment.
small nit: is_collapsible is generally superfluous, just collapsible is fine throughout
compile_assets/scss/admonition.scss
Outdated
| @@ -186,6 +188,30 @@ a.admonition-anchor-link { | |||
| } | |||
| } | |||
|
|
|||
| summary { | |||
There was a problem hiding this comment.
We should be specific we're only styling our summaries here, there could be others on the page in user content.
For example:
| summary { | |
| summary.admonition-title { |
compile_assets/scss/admonition.scss
Outdated
| transition: transform .25s; | ||
| } | ||
|
|
||
| details[open] > &::after { |
There was a problem hiding this comment.
| details[open] > &::after { | |
| details[open].admonition > &::after { |
| --md-details-icon: | ||
| url("data:image/svg+xml;charset=utf-8,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 24 24'><path d='M8.59 16.58 13.17 12 8.59 7.41 10 6l6 6-6 6-1.41-1.42Z'/></svg>"); |
There was a problem hiding this comment.
I think this looks perfect! Thanks very much.
For reference, we have:
and
|
Alright, updated and squashed. Just waiting for CI to run. The final step was adding some docs and unit/integration tests (as we're adding new html elements I wanted to check that interaction too). |
This Pull Request adds collapsible admonition support, as described in #18.
In its current state, this implementation has 2 shortcomings that I'm not sure how to overcome: