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

adding cell tag data attributes to HTML exporter #1090

Merged
merged 3 commits into from
Aug 28, 2019

Conversation

choldgraf
Copy link
Contributor

@choldgraf choldgraf commented Aug 19, 2019

In Jupyter Book most of my custom templates exist basically to insert the contents of cell tags into the <div>s metadata. I was thinking maybe this is something that would be useful to the HTMLExporter in general.

This PR modifies the base HTML template to do the following:

If a cell has any tags defined, then it adds a data-cell-tags attribute to the code or markdown cell. this attribute has a comma-separated string of tags.

This could then be used with selectors etc to do various things to the output HTML based on the tags content.

I thought about doing this with classes instead of data- attributes, but it seemed that the former might cause unexpected behavior. If folks think this PR is a good idea, but that using classes would be better, I'm happy to revisit that.

(I can also add a test for this if folks are +1 on it)

@MSeal
Copy link
Contributor

MSeal commented Aug 21, 2019

I also think this is a good idea. I'd be on-board after a test or two to cover the feature. I think I agree with your assessment of using data- attributes

@choldgraf
Copy link
Contributor Author

I added a test to make sure the tags make it into the HTML as expected. Let's see if travis is happy!

@choldgraf
Copy link
Contributor Author

A quick thought here: what do you think about instead of data tags, adding these as classes but with a short prefix to make sure they don't clash with anything else. Something like celltag_{tagname}?

The reason I'm rethinking this is because if people are writing CSS rules using these tags (spoiler, I might be one of these people 😄), it can be more complex to select subsets of data tags etc, while it is straightforward to use tags.

Do you have any thoughts on that @MSeal? Happy to discuss...

@MSeal
Copy link
Contributor

MSeal commented Aug 27, 2019

Class paths with a prefix seems fine by me as well. If you think this would work better for the practical use-case go for it. The celltag_ prefix seems like a safe choice as a prefix.

@choldgraf
Copy link
Contributor Author

Cool - see the latest version for what this looks like, lemme know what you think!

Copy link
Contributor

@MSeal MSeal left a comment

Choose a reason for hiding this comment

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

LGTM, tried it out on some sample notebooks

@MSeal MSeal merged commit aeaa214 into jupyter:master Aug 28, 2019
@choldgraf
Copy link
Contributor Author

🎉

@MSeal MSeal added this to the 5.6.1 milestone Oct 21, 2019
@MSeal MSeal added this to the 5.6.1 milestone Sep 8, 2020
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