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

Hide buttons from TryExamples directive by default #173

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

agriyakhetarpal
Copy link
Collaborator

Description

This PR hides the buttons from the TryExamples directive by default. This is helpful because the current method of ignoring particular pages based on their URL first loads the buttons and then hides them, which means that they are briefly displayed on that page in the documentation and then hidden – keeping them hidden and then selectively enabling them will minimise this latency and improve user experience.

Changes made

  1. Made all try_examples_button buttons hidden at the time of generation by adding this keyword to the classList
  2. Updated the loadTryExamplesConfig function in jupyterlite_sphinx.js to check for the try_examples_button hidden class instead and then remove hidden from the classList

Additional context

N/A

@agriyakhetarpal agriyakhetarpal force-pushed the make-buttons-hidden-by-default branch from d63d2e5 to 0ed1b1b Compare May 1, 2024 16:47
@agriyakhetarpal agriyakhetarpal force-pushed the make-buttons-hidden-by-default branch from 0ed1b1b to 09d48ab Compare May 1, 2024 16:48
@agriyakhetarpal
Copy link
Collaborator Author

I have rebased this right now and opened a PR because I think it will be better for visibility, plus I would appreciate some help towards bringing this to completion. It's not fully working right now since the directives still seem to contain the try_examples_button hidden and therefore are not displayed, even when I try to remove the keyword from the classList. It has been a while since I worked on this, so I'm not sure what I was debugging back then.

Copy link
Collaborator

@Carreau Carreau left a comment

Choose a reason for hiding this comment

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

I see why this is reasonable, but isn't that going to have the opposite effect on page where the button should be visible, where the page will load and then jump around as buttons are added ?

.gitignore Outdated Show resolved Hide resolved
jupyterlite_sphinx/jupyterlite_sphinx.js Outdated Show resolved Hide resolved
@agriyakhetarpal
Copy link
Collaborator Author

I see why this is reasonable, but isn't that going to have the opposite effect on page where the button should be visible, where the page will load and then jump around as buttons are added ?

Thanks for the review, @Carreau. Yes, this will be a slight issue when someone reloads a page and if they are in the middle of a page when doing so – but at the top of the page, it should load as normal, because from there the buttons will be appended to the layout rather than being inserted into a position where the text moves because of the insertion. I am not sure if there is a cleaner way to do this.

@agriyakhetarpal
Copy link
Collaborator Author

I think this is partially working now – where it breaks is when a try_examples.json config file is not present, and therefore data.ignore_patterns will always be false. I'll try to move the logic for un-hiding the buttons into a separate function and then call it. This way, the currently hidden buttons can be un-hidden with or without the presence of the configuration file.

@agriyakhetarpal agriyakhetarpal changed the title [WIP]: Hide buttons from TryExamples directive by default Hide buttons from TryExamples directive by default May 2, 2024
@agriyakhetarpal
Copy link
Collaborator Author

agriyakhetarpal commented May 2, 2024

I think this is working now, marking it as ready for review. One small thing to figure out remains: the PR preview at https://jupyterlite-sphinx--173.org.readthedocs.build/en/173/directives/try_examples.html is not un-hiding the button with the custom blue-bottom CSS present in jupyterlite_sphinx.css. The button is visible in the stable documentation: https://jupyterlite-sphinx.readthedocs.io/en/stable/directives/try_examples.html

So this works for all sorts of buttons now (tested locally), except for these non-standard buttons that have user-defined CSS. Let me try to dig further, and meanwhile open this up for further comments.

@agriyakhetarpal agriyakhetarpal marked this pull request as ready for review May 2, 2024 18:18
@jtpio jtpio added the enhancement New feature or request label May 3, 2024
@Carreau Carreau self-assigned this Jul 22, 2024
@Carreau
Copy link
Collaborator

Carreau commented Jul 22, 2024

I will try to re-review.

I'm ok with this in principle. Still unsure if hiding or appending is the best, so I'd like someone else input on this anyway.

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

Successfully merging this pull request may close these issues.

3 participants