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 support for tabs / tabset panels #571

Merged
merged 6 commits into from
Apr 5, 2024

Conversation

astroDimitrios
Copy link
Contributor

@astroDimitrios astroDimitrios commented Feb 7, 2024

What: This pull request adds support for tab and group-tab admonitions/panels. They are based off sphinx-tabs. It does this by adding an extra lua filter.

Why: So I can have a tab of instructions for each computer platform which sync (group-tabs) across episodes. Tabs are also useful when teaching compiled languages as I can have a tab for each compiler.

Syntax:

tab

::: tab

# gnu

1

# Intel

2

# Craaaay

This is some text before the code block.

```fortran
  print *, 'WHAT is Cray today'
```

Text after

:::

Output:

tab

group-tab

::: group-tab

# Linux

1

# Windows

2

# Archer

3

:::

::: group-tab

# Linux

4

# Windows

5

# Archer

6

:::

Output:

group-tab

The underlying functionality is all handled by bootstrap tabs. I have used the resources mentioned in carpentries/varnish#32 to get the aria labels right and the JS is based of sphinx-tabs.

Caveats

  • Only text, images, and code blocks can be inside a tab. I tried getting the filter to accept other callouts inside a tab. Since any nested callout is processed first and the other callouts (say spoiler) are made up of any number of divs and other elements inside I couldn't find a way to soley capture the main 'spoiler' div for instance to go inside a block. Contrast this with a code block which is processed as one single div element by the filter and can easily be inserted in the tabpanel. I tried writing the filter so that it walked inlines not blocks and just couldn't get other callouts to work inside tabs. For my use case I don't see this as a huge issue but maybe others have different use cases.
  • Use of global lua variables - I had to add a few more. The filter would normally process and return elements in order so header 1, then content 1 for the first tab etc. I wanted to collate all the headers for the nav tablist and collate all the tabcontent to go below the nav as in the bootstrap example (the second one in this section using . That meant storing the buttons and content in lua tables so they are separate and wrapping them in divs after the main filter has run.
  • I have probably missed adding tests, and I tested the built lesson on Firefox and Chrome.

I have added a lot to the lua so I'm happy to have a zoom etc chat if anyone is looking at the code as part of a review.

To test this change you will need the corresponding varnish and pegboard changes:

First attempt at adding tabs to the lua code
Fix the id's of the buttons and the tabpanels so that the buttons control the correct tabpanel.
Modify the lua filter for the tab block to also create group-tabs.
Add in extra comments for the group tab filter.
@ErinBecker
Copy link
Contributor

Thank you for this PR @astroDimitrios, and for the excellent description and documentation. I do see this feature being very useful, especially for our workshop setup pages (for example https://datacarpentry.org/ecology-workshop/setup-python-workshop.html), which are currently using the spoiler callout class to hide different setup options. I know @froggleston responded in Slack to let you know we'll be taking a look at this in the next couple of weeks and wanted to also jump in to say I'm excited to dig in and test this new feature!

@ErinBecker
Copy link
Contributor

@astroDimitrios - Thanks for your patience. I've finally had some time to play around with this and, although I'm able to get this up and running, I am seeing some odd behavior that I'm wondering if we could change.

  1. The tab callout requires tab names to be prefixed by a single #, whereas all of our other callouts require titles to use double ##. This is likely to be confusing for folks trying to migrate over existing callouts to the tab style. Is it possible to change this so that the new callout type uses ## for tab titles?
Tab tiles with single hashtag display.

In the above screenshot, the "Windows" tab title appears. It is prefixed by a single hashtag in the markdown file.

Tab titles with double hashtag do not display.

In the above screenshot, the "Windows" tab title does not appear. it is prefixed by a double hastag in the markdown file.

  1. Content formatted as a list (either ordered or unordered) either does not appear at all, or appears with list formatting removed.
Content formatted as unordered list does not appear in callout.

In the above screenshot, content formatted as an unordered list does not appear at all in the tab callout.

Unordered list has list formatting removed.

In the above screenshot, content formatted as an unordered list appears, but without list formatting.

Mixed ordered and unordered list items do not render as expected.

In the above screenshot, numbered list items appear (with list formatting removed) and unordered list items do not appear at all.

Installation instructions are likely to be a common use case for tabbed callout boxes, so the ability to render lists (and especially ordered lists) seems pretty important. Do you think there's a way to incorporate this?

  1. This item is, in my opinion, less important than the two above, but would be nice from a styling perspective. When there are three or more tabs, there is no graphical distinction between the unselected tabs. This may introduce visual confusion for users. Would be interested to hear others' thoughts on whether this is an issue we should address.
Unselected tabs have no visual separation.

I'd personally be pretty excited to get this new feature incorporated into the Workbench, as I think it's a pretty big improvement over our current method for showing installation instructions across different platforms using spoiler class callouts (example below).

Example of operating instructions for different platforms split across multiple collapsed callout boxes.

Do you think it would be possible to incorporate changes mentioned in items 1 and 2 above? @froggleston and I can take a look at the styling (item 3) if that's outside of your wheelhouse. Not that it's in our wheelhouse either! 😂

@astroDimitrios
Copy link
Contributor Author

@ErinBecker

  1. This is an easy thing to change (1 line of code and 1 comment in the Lua filter). I checked the component guide and the callouts/spoiler seem to use ### instead of ##. Let me know if you want it to be ## instead.

New syntax:

::: tab

### Windows

- List one
- List 2
- List 3

After list

:::
  1. Oops! Completely forgot about lists - I have modified the Lua and they pull through fine now.
  2. I have added text-decoration to the non-active tabs (in Varnish). Let me know what you think.

Screenshot below showing altered styling and an unordered list:
image

Screenshot below showing an ordered list:
image

@froggleston froggleston merged commit 3b41671 into carpentries:main Apr 5, 2024
12 checks passed
@ocaisa
Copy link
Contributor

ocaisa commented Apr 5, 2024

@astroDimitrios I just wanted to understand the functionality of group tabs, does that mean if I make a selection for one set of tabs, the same selection is made for the other set(s).

The reason I'm asking is that I would have liked to make the default tab configurable (so I could choose between a SLURM tab and a PBS tab in the configuration file for my use case). I could live without that though if they only had to select the tab once and the choice would propagate through.

@astroDimitrios
Copy link
Contributor Author

@astroDimitrios I just wanted to understand the functionality of group tabs, does that mean if I make a selection for one set of tabs, the same selection is made for the other set(s).

The reason I'm asking is that I would have liked to make the default tab configurable (so I could choose between a SLURM tab and a PBS tab in the configuration file for my use case). I could live without that though if they only had to select the tab once and the choice would propagate through.

Hi @ocaisa for group tabs clicking once on say the Windows tab turns all other group tabs to windows and will remember the users choice :)

@astroDimitrios astroDimitrios deleted the feature/tab-admonition branch April 5, 2024 16:49
@froggleston
Copy link
Contributor

Thank you so much again for your contribution @astroDimitrios ! You can see your tabs in the wild in the sandpaper docs 😄 https://carpentries.github.io/sandpaper-docs/episodes.html

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.

None yet

4 participants