Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dropdown directive #14

Closed
choldgraf opened this issue May 15, 2020 · 18 comments · Fixed by #15
Closed

Add dropdown directive #14

choldgraf opened this issue May 15, 2020 · 18 comments · Fixed by #15

Comments

@choldgraf
Copy link
Member

We've discussed this in a few other spots, but figured it's worth making an issue about it.

Right now we're using custom JS/CSS in the sphinx-togglebutton extension, but we could probably use some combination of

for this, especially for the "toggle buttons with titles" parts of sphinx-togglebutton

If we implement that functionality here, then I am fine deprecating sphinx-togglebutton (though at that point we may wanna rename this to sphinx-bootstrap or something).

The only thing we need to make sure of is that we don't mess-up the CSS styling of togglebuttons...we don't want those elements to take up too much visual space IMO

@chrisjsewell
Copy link
Member

chrisjsewell commented May 15, 2020

Way ahead of yah 😉

Basically I am going to copy https://github.com/tk0miya/sphinxcontrib-details-directive/blob/master/sphinxcontrib/details/directive/__init__.py, but adapt it with some bootstrap (cards) + custom CSS, as exampled below.
It works well (tested on Firefox/Chrome/Safari), and a big pro is that in requires no JavaScript.
Interestingly it's what GitHub uses a lot: https://docs.google.com/presentation/d/1hvnPpsJo44BTPfJx28CV95vqk_dt6na1awUbk0kmZYM/edit#slide=id.g3e31444916_0_103

<style>
  details.sphinx .summary-title {
    -webkit-user-select: none;
       -moz-user-select: none;
        -ms-user-select: none;
            user-select: none;
    padding-right: 3em!important;
  }
  details.sphinx:hover {
    cursor: pointer;
  }
  details.sphinx .summary-content {
    cursor: default;
  }
  details.sphinx summary {
    padding: 1em;
    list-style: none;
  }
  /* chrome doesn't yet support list-style */
  details.sphinx summary::-webkit-details-marker {
    display: none;
  }
  details.sphinx summary:focus {
    outline: none;
  }
  details.sphinx summary:hover .summary-chevron-up svg {
    opacity: 1;
  }
  details.sphinx .summary-chevron-up svg {
    opacity: 0.5;
  }
  details.sphinx .summary-chevron-up,
  details.sphinx .summary-chevron-down {
    pointer-events: none;
    position: absolute;
    top: 0.75em;
    right: 1em;
  }
  details.sphinx .summary-chevron-up svg,
  details.sphinx .summary-chevron-down svg {
    display: block;
  }
  details.sphinx[open] .summary-chevron-up{
    visibility: hidden;
  }
 </style>

<details class="sphinx card">
  <summary class="summary-title card-header bg-danger text-white">
    Details element with custom arrow!
    <div class="summary-chevron-up">
      <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><polyline points="6 9 12 15 18 9"></polyline></svg>
    </div>
  </summary>
  <div class="summary-content card-body">
    <p class="card-text">paragraph 1</p>
    <p class="card-text">paragraph 2</p>
  </div>
  <div class="summary-chevron-down">
    <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><polyline points="18 15 12 9 6 15"></polyline></svg>
  </div>
</details>

@chrisjsewell
Copy link
Member

Give it a go and let me know what you think

@choldgraf
Copy link
Member Author

choldgraf commented May 15, 2020

oh nice! when you say "give it a go", do you mean this: https://github.com/tk0miya/sphinxcontrib-details-directive/blob/master/sphinxcontrib/details/directive/__init__.py ?

So basically, you're suggesting "use the HTML details element with styling so that it doesn't look crappy"? If that's it, then I think it's a great idea :-)

@chrisjsewell
Copy link
Member

chrisjsewell commented May 15, 2020

Yeh literally just copy/paste the HTML above into a page (that has bootstrap CSS loaded), and you should see:

image

@choldgraf
Copy link
Member Author

nice - looks good to me (though the extra "button" styling is missing in my demo). I suppose that themes etc could also control some styling. The toggle-buttons in sphinx togglebutton are also pure CSS (even though toggling the actual content uses js)

@chrisjsewell
Copy link
Member

though the extra "button" styling is missing in my demo

What does it look like to you? Do you have e.g. <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/css/bootstrap.min.css" integrity="sha384-Gn5384xqQ1aoWXA+058RXPxPg6fy4IWvTNh0E263XmFcJlSAwiGgFAW/dAiS6JXm" crossorigin="anonymous"> also in the demo

@choldgraf
Copy link
Member Author

choldgraf commented May 15, 2020

ah I just missed your point about "that has bootstrap css loaded" haha, I literally copypasted into a local html file and opened it in chrome. It looks great in a page built with jupyter book ✨

@choldgraf
Copy link
Member Author

choldgraf commented May 15, 2020

so you're imagining that with things like cells, we just wrap their content in a details element via this directive? One thing to think about is what to do about content that doesn't use the "title" in the details. Maybe that's out-of-scope for this directive though?

@choldgraf
Copy link
Member Author

choldgraf commented May 15, 2020

another note - perhaps we could allow users to specify Sphinx admonition styles rather than using the bootstrap-specific class names. E.g. to let people do:

```{dropdown} My title
:style: warning
content
```

and that would adopt the styling of the sphinx {warning} block

@chrisjsewell
Copy link
Member

things like cells.. Maybe that's out-of-scope for this directive though?

Yeh thats a bit more tricky. I think for details there has to be at least some kind of element above the content that you're hiding, as opposed to the button to the side that you have now.
It doesn't have to be a whole banner though, e.g. the emoji buttons on this page uses details

@choldgraf
Copy link
Member Author

yeah that makes sense - in the case of togglebutton, the element could just be the "+/-" or the "v ^" elements to the right

@chrisjsewell
Copy link
Member

chrisjsewell commented May 15, 2020

perhaps we could allow users to specify Sphinx admonition styles rather than using the bootstrap-specific class names

We can talk about this more when I do the PR (soon).
But I guess this would define the defaults for classes on each element; details, summary and the "body" div below summary. Then additionally each one could be (optionally) amended separately, as is done with panels. Something like:

```{dropdown} My title
:style: warning
:main: + p-2
:title: + text-center
:body: + text-align

content
```

@choldgraf
Copy link
Member Author

yeah - I think it's a broader question for this repo (or our general "bootstrap support") whether we want users to use bootstrap-specific classes etc, or if we want to piggy-back on the Sphinx terminology. I think there are pros/cons to each. But happy to iterate in a PR!

@chrisjsewell
Copy link
Member

piggy-back on the Sphinx terminology

What do you mean by this? Just note, warning, attention, etc?

@choldgraf
Copy link
Member Author

choldgraf commented May 15, 2020

yeah, like do we ask people to put bg-danger or do we ask them to put warning

FWIW there are a bunch of these translations here: https://github.com/pandas-dev/pydata-sphinx-theme/blob/master/pydata_sphinx_theme/bootstrap_html_translator.py

@chrisjsewell
Copy link
Member

chrisjsewell commented May 15, 2020

That type of translation only really makes sense for single element constructs.

Just use the bootstrap class names for setting specific classes; they are well named and well documented already.

But then, as I mentioned, there could be "top-level" styles option that initialize a bunch of bootstrap classes (maybe on multiple elements), i.e. warning wouldn't just relate to bg-danger; it might be e.g. font-weight-bold text-white bg-danger on the detail element, then potentially also other classes on the summary and div elements.

@chrisjsewell
Copy link
Member

Also I'll probably create a dropdown-group directive; primarily in order to set the top/bottom/inter-dropdown spacing.

@choldgraf
Copy link
Member Author

that all makes sense to me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants