Skip to content

Conversation

@rkdarst
Copy link
Contributor

@rkdarst rkdarst commented Dec 27, 2021

  • This was inspired by the discussion in Improve toggle button design and structure #28, and having a bit of
    spare time and wanting to try something. This gets the "admonition"
    part of those design ideas.

  • In the javascript setup, also configure the whole admonition title
    element to be clickable. This happens only in the case where it is
    a direct descendent of the admonition box and there is only one
    admonition title descendent.

  • This is not designed to be perfect, but is designed to be mostly
    safe with most themes. It still could cause problems, but it has
    been tested with alabaster, sphinx_book_theme, and sphinx_rtd_theme.

  • I don't claim to know javascript or CSS, or have deep knowledge of
    sphinx-togglebutton. But I do know enough and could read and web
    search enough to see how to add the onclick watcher + data to
    identify the right thing to toggle.

  • Suggested reviews:

    • Someone that knows sphinx-togglebutton should see if this makes
      sense.
    • Someone that knows javascript should see if this makes sense
    • It would be prudent for someone else to test (I've tested the
      three themes above and firefox/chrome)
    • But if no one can test the above, it seems fairly safe to me and
      could be released and fixed later.

- In the javascript setup, also configure the whole admonition title
  element to be clickable.  This happens only in the case where it is
  a direct descendent of the admonition box and there is only one
  admonition title descendent.

- This is not designed to be perfect, but is designed to be mostly
  safe with most themes.  It still could cause problems, but it has
  been tested with alabaster, sphinx_book_theme, and sphinx_rtd_theme.
@rkdarst rkdarst changed the title IMPROVE: Allow clicking on whele admonition title IMPROVE: Allow clicking on whole admonition title Jan 25, 2022
@rkdarst
Copy link
Contributor Author

rkdarst commented Jan 25, 2022

Just wondering if this was noticed (I submitted it during the holiday times)

Copy link
Member

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

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

Ooh I did miss this! But I think that it's a nice addition! Ideally we could do this purely with CSS, but I think adding the JS functionality is a nice step forward nonetheless.

I added a quick additional CSS rule so that the mouse becomes a "pointer" when you hover over toggle-able admonition titles, but other than that this seems good to go. I also removed some specific selectors for div so that this works with newer versions of sphinx/docuils

@choldgraf
Copy link
Member

Let me know if my commits look good to you @rkdarst - then we can merge 👍

@choldgraf choldgraf merged commit eb21421 into executablebooks:master Feb 2, 2022
@choldgraf
Copy link
Member

many thanks for this improvement @rkdarst !

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 this pull request may close these issues.

2 participants