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

👌 IMPROVE: Replace sphinx-togglebutton with built-in functionality #446

Merged
merged 1 commit into from Sep 29, 2022

Conversation

chrisjsewell
Copy link
Member

@chrisjsewell chrisjsewell commented Sep 29, 2022

This allows for tighter integration with myst-nb:

  • Nicer rendering of the hidden buttons
  • Customisation of the hide/show prompts

The implementation uses "static" details/summary HTML tags directly + CSS,
similar to sphinx-design,
rather than the JS implementation used by sphinx-togglebutton.

This allows for tighter integration with myst-nb:

- Nicer rendering of the hidden content
- Customisation of the hide/show prompts

The implementation uses "static" details/summary HTML tags directly + CSS,
similar to sphinx-design,
rather than the JS implementation used by sphinx-togglebutton.
@codecov
Copy link

codecov bot commented Sep 29, 2022

Codecov Report

Base: 81.37% // Head: 81.74% // Increases project coverage by +0.37% 🎉

Coverage data is based on head (c466e93) compared to base (4dcf7c5).
Patch coverage: 98.52% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #446      +/-   ##
==========================================
+ Coverage   81.37%   81.74%   +0.37%     
==========================================
  Files          29       29              
  Lines        2572     2630      +58     
==========================================
+ Hits         2093     2150      +57     
- Misses        479      480       +1     
Flag Coverage Δ
pytests 81.74% <98.52%> (+0.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
myst_nb/core/render.py 85.56% <95.45%> (+0.39%) ⬆️
myst_nb/core/config.py 79.06% <100.00%> (+0.32%) ⬆️
myst_nb/sphinx_.py 91.66% <100.00%> (+1.53%) ⬆️
myst_nb/sphinx_ext.py 91.48% <100.00%> (-0.35%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@chrisjsewell
Copy link
Member Author

@chrisjsewell chrisjsewell merged commit bd852a0 into master Sep 29, 2022
@chrisjsewell chrisjsewell deleted the replace-togglebutton branch September 29, 2022 13:02
@choldgraf
Copy link
Member

choldgraf commented Sep 29, 2022

Please give others a chance to review PRs, especially if we are changing UI/UX, and especially if there is not a linked issue that already has discussion and agreement on a change - at least one global working day unless it's something really mission critical to merge quickly.

This seems OK to me - it's pretty nice to be able to configure the text and to have code cell-specific text.

Two design comments:

sharp edges: The "rounded corners on top and sharp edges on bottom" UI feels a little bit awkward to me - my vote would have been to just style these like regular buttons rather than have the sharp edges. This feels clunky in particular in the output cells:

image

That said, I'm not a UI/UX expert so this is something we should just get feedback from others on over time.

a11y: I suspect that the color contrast of the font isn't strong enough for accessibility / vision-impaired people. Can we increase the contrast of the text?

image

@chrisjsewell
Copy link
Member Author

Heya @choldgraf I did indeed need it for an actual live demonstration I gave yesterday 😅 https://aiida-qe-demo.readthedocs.io/en/latest/2_bands_workflow.html

already has discussion and agreement on a change

The problem is there was no real agreement on sphinx-togglebutton in the first place.
As you know, I wasn't happy with the initial implementation (using the ⊕ buttons), and we discussed offline how to adapt that to essentially use my implementation from sphinx-design (executablebooks/sphinx-togglebutton#32)

This is better, but still when you actually use this in practice, you end up with many

image

which is absolutely not clear to the user what it actually is, until they click it

with the new implementation (post #450) I'm really happy with the UI:

image

(expanded)
image

It is now clear, from both the green margin and the prompt, that these are all related to code cells.

For

image

the goal of the UI is to indicate that this is only the top of the code cell, and that clicking on it will reveal the rest

@choldgraf
Copy link
Member

choldgraf commented Oct 5, 2022

I think we can discuss elsewhere, but the point isn't to debate the specific UX of this implementation (the "sharp edges" approach has grown on me and I think it looks nice as well). I think these are the two main points:

  • For decisions, we need visibility across the team and community before a decision has been made. Ideally each PR should be linked to an issue that provides the context and rationale for needed changes. People (especially non-developers) need to be able to provide feedback about UI/UX that doesn't require coding knowledge. We don't need full consensus to decide, but decisions shouldn't be made ad-hoc and unilaterally. Others at least need a chance to observe the rationale and object if they wish.
  • For implementations, we need to give time for others on a team to review. This helps spread knowledge about the codebase across multiple people to prevent information silos. It also generally will lead to changes that are broadly acceptable to the team of people working on a repository.

Obviously no project follows both of these perfectly, but we need to begin following practices like this more closely as we grow these projects beyond a small grant-funded team.

To your point above - we didn't follow this process for sphinx-togglebutton, and as a result there was disagreement in the end-result that probably could have been smoothed out earlier on and saved everybody some time. I moved forward quickly there for the same reasons you did here - we needed to iterate and get something out quickly. But we don't want to keep repeating these anti-patterns in the future if we want to grow a community around these tools. My point isn't criticize but to start nudging our team towards healthier distributed practices.

@chrisjsewell
Copy link
Member Author

My point isn't criticize but to start nudging our team towards healthier distributed practices.

yep I completely agree on all of this, and thats why you need to stop doing anything else you're doing on EPB and make your MEP (https://github.com/executablebooks/myst-eps) on the steering council 😉

@choldgraf
Copy link
Member

choldgraf commented Oct 5, 2022

Just to be clear, in my opinion we don't want a steering council to make UI/UX decisions about individual repositories in the organization. I think it's important to delegate and distribute our decision-making so that many decisions are made close to the place where the work will happen. The steerco is meant to define strategy, policy, and steward the community. In this case, I think a steering council would decide on the policy for "what process to follow when gathering community feedback for decisions", not on the decision about whether to use sharp edges on code blocks. Fortunately I think that we can start to improve our policies and practices without waiting for a steering council to make a formal proposal.

@chrisjsewell
Copy link
Member Author

I but it defines the top level process of how this is done, and also probably top level goals design decisions for UI/UX. both of which can be refer to when we have these conversations

@kai-tub
Copy link

kai-tub commented Oct 5, 2022

Hey, sorry for adding to the off-topic conversation, but I would like to think that I have something to add.
I am one of those silent followers that closely observe the development of this project (and all other projects related to MyST and furo/STB) because I think they have one of the highest impact potentials for the scientific and education communities (with nbdev2 also on the top of the list).

But enough about me, I actually did see that this PR was opened and also had the same thought as @choldgraf about the sharp edges and the color contrast.
I also thought about possibly accessibility implications due to the aria-role overwriting (see here for example) of the summary/details tags but couldn't really come up with a solid reason not to use it for now. I would only assume that if the output is rendered as Markdown/HTML text that should be accessible, it could fail.

So I would like to argue that it is nice (at least for me) if one can follow the decision process of UI changes or any updates at all really.

[...] This helps spread knowledge about the codebase across multiple people to prevent information silos. It also generally will lead to changes that are broadly acceptable to the team of people working on a repository.

Obviously no project follows both of these perfectly, but we need to begin following practices like this more closely as we grow these projects beyond a small grant-funded team.

I would also like to piggy-back on this comment.
From an (interested) outsider's perspective, there are a couple of challenges that have led me to follow the project and not contribute to it.

But before I give the wrong impression, please understand that I am very grateful for all of your work and that I am a huge fan of the executable book project, and it would be a dream come true to work on executable book projects via grants. 😄

Sorry, the text became A LOT longer than I thought... 😅

Kai's way too long feedback

External project experience

To keep it real, I would like to talk about a thing I've implemented:

I've implemented a custom preprocessor to parse the notebooks and to inject tags via code comments (nbdev style since I personally think it is an important feature when coming from nbdev).
Now I wrote about it in the show and tell category of the discussion forum.
The implementation was my own solution to an earlier feature request I've made.
I asked about a possible 'community' page on MyST-NB to allow related projects/solutions to reach a wider audience, but there was no follow-up on this.
Of course, I should've created a new issue about this topic, and I am being selfish here because maybe the project is just uninteresting for the wider audience and doesn't justify creating a custom 'community' page, but it feels like a chicken and egg problem.
If the page (even if it is empty) exists, it would at least make the project seem more open.
Again, I am not writing this because I would like to get more attention (or for my project to get more attention), but it did make me feel like the community is not as open as I think it really is.

TL;DR: I think every executable books project page should have a community page where external projects can be advertised to minimize duplicated work and let developers feel like they are giving back and not just solving their problems in a public repo.

Discussion Forum experience

And as I also mentioned the "Discussion Forum", I would also share my personal experience:
I frequently forget that it exists and don't really follow what happens inside of it. Even if I am notified about almost every PR (or at least release) from the executable books project.
Maybe it is just me, but I think the discussion forum should be more prominently displayed/advertised on all documentation pages and more often mentioned.
Again, I am just talking about my experience, but I don't feel inclined to contribute to the discussion forum because I frequently forget about it and because it doesn't feel like a 'relevant' part of the community.
It feels like a place where more general questions are asked but still mostly answered by the maintainers (speaking generally).
Maybe this is a general issue with GitHub discussions but I would like to think that if the Forum is better highlighted or if there is some way to incentive it that more people would help out with answering.
Sorry for going off on a tangent, especially since I am not actively following the Discussion Forum but I thought it could be relevant feedback.

TL;DR I personally keep forgetting that the GitHub Discussion Forum exists. Maybe more advertising could help? Or better communicating what can be asked in the forum and that answering those questions is a valuable task?

Project splintering

Another issue I have is that the organization has many documentation websites and that it is hard to communicate external projects to the end-user.
To continue to babble about my project (sorry again):
Let's assume that the project is added to a community page on MyST-NB.
I feel like it should belong to this 'documentation' website because it is specific to MyST-NB and doesn't require the full jupyter-book library.
But I assume that most users that would benefit from it would come from the jupyter-book community and not from the MyST-NB only users.
But then the community page would have to be duplicated (especially since this requires changes to the set-up).
As an external contributer, I would be hesitant to ask at two different projects to be included on two community pages.
Maybe there is a way to "inherit" community pages from the lower stack and adding some comments to it?
I don't have a solution to it, but it becomes even worse I would like to share a custom MyST-role (I think).
Also, I don't think it should be added to the meta-project because I also find this way too hard to find when you are only interested on either jupyter-book or MyST-NB.
It is generally hard as a new-comer to manage all of the different levels.
Maybe a single "If you are looking for X jump to Y" could help? But I know that this is an inherent issue of complex applications.

TL;DR: Due to the many projects under the executable books umbrella, it is hard to work on a 'lower-level' and see how external work can be 'advertised' to the end-user. I have no solution but I think it should be discussed. Especially when we start to draw a line between "implementation details" of dependencies like MyST-Parser/Sphinx. Maybe every page could have a toolchain section that "visualizes" the different levels (like visual style of jupyter book? Maybe videos can help? Like I said, I don't know.

Large tech-stack

IMHO the above points also lead to a high entry barrier to contributing to the MyST-NB/Jupyter-Book repositories.
And I know that it is a complex topic, but there are so many components/moving parts it is hard to know where to start.
How can I implement my own summary/details logic? How can I create custom roles?
Don't get me started on Sphinx and how long it takes to understand creating roles for Sphinx and how to test them.
That in it self would be extremely helpful. Or maybe it is not even necessary anymore, since 0.15 or 0.16?
This team has gained a lot of experience in various different tools that I would consider not 'beginner' friendly and I think it would be helpful to provide some mini-tutorials to extend or customize parts of Jupyter Book or MyST-NB.
The idea to create a tutorial for the AST part was also proposed by @choldgraf at some point during the decouple rewrite from Sphinx.
On this note, I would also like know if this is a sign for the move away from Sphinx? As a contributor, I would also like to know where the project is moving in the long-term. But I guess that will be solved with EBP :)

TL;DR: Jupyer-Book is a very complex piece of software. To make it easier for contributors, I think there is a need for more 'developer' specific tutorials. Maybe this could be crowd-sourced, but then it should be 'advertised/endorsed' by the executable books forum. But especially the low-level stuff (Sphinx/testing related) is something I think many are missing the expertise on and is something that would best be tackled by the maintainers themselves.

Summary

Sorry for writing so unbelievably much. I didn't intend to write all of these details 😅
There are more things I would like to add, but I think this is getting way too long, so I will stop it here.
Again, I am a big fan of the executable books project and would just like to give you my personal outside perspective :)
I just wrote soo much because I care soo much about this project ❤️

Cheers!

@choldgraf
Copy link
Member

Thank you for this feedback

Kai - I want to express my strong appreciation for this feedback. It is really hard to notice if a project is hard to contribute to, precisely because you don't tend to talk to the people that aren't present. I think one of the most important questions an open project can ask is "who is not in the room right now, and why not?"

Your post provides a number of helpful clues and ideas as to why others may not be able to contribute. As we begin to move the project beyond its initial grant-funded team, I think this will be crucial to improve.

My opinion is that if you, a highly-engaged person with development capabilities, feel this way, then there are many, many more people who feel the same way. I'm going to provide a few quick responses below - some of them are explanations for points you make but I just want to re-iterate that these aren't making excuses - I recognize that they are all major aspects of growing a community that we must improve.

So I would like to argue that it is nice (at least for me) if one can follow the decision process of UI changes or any updates at all really.

I think this is crucial. We must separate the discussion and decision-making process from the implementation process. If we don't do this then it will be very hard for non-developers (or really, non maintainers) to participate in these processes.

We have had some discussion of this in the issue below, and I've encoded your suggestions in a comment there there as well. Please provide feedback there if you'd like to see any refinements!

TL;DR: I think every executable books project page should have a community page where external projects can be advertised to minimize duplicated work and let developers feel like they are giving back and not just solving their problems in a public repo.

This is an interesting idea - you think it'd make more sense to delegate this to each sub-project rather than have one centralized place for it?

TL;DR I personally keep forgetting that the GitHub Discussion Forum exists. Maybe more advertising could help? Or better communicating what can be asked in the forum and that answering those questions is a valuable task?

Agreed - one way that we could resolve this is by using a standardized theme across all of the repositories. This is similar to what Dask does. That way they have the same top-level links on any of their documentation pages. Either way, I agree that we should make it clear where to have general discussion in each repository. I've opened an issue to track this here:

Especially when we start to draw a line between "implementation details" of dependencies like MyST-Parser/Sphinx. Maybe every page could have a toolchain section that "visualizes" the different levels (like visual style of jupyter book? Maybe videos can help?

Totally agree - at least we could have a centralized place that explained the whole stack and all of its components. This is something that @mmcky has been interested in doing as well, and I know that @rowanc1 likely benefitted from conversation like this as the curvenote team was onboarding as well.

I've encoded this suggestion in this issue, please suggest there if we should refine something:

I think there is a need for more 'developer' specific tutorials. Maybe this could be crowd-sourced, but then it should be 'advertised/endorsed' by the executable books forum. But especially the low-level stuff (Sphinx/testing related) is something I think many are missing the expertise on and is something that would best be tackled by the maintainers themselves.

Also agreed here. I believe this is another thing that @mmcky has been interested in for a while now. Here's one issue where he's tracking this:

I think that your suggestion here is slightly more specific, so I've also encoded it in this issue here:

I would also like know if this is a sign for the move away from Sphinx? As a contributor, I would also like to know where the project is moving in the long-term. But I guess that will be solved with EBP

Good question! Folks on the EBP team are trying to figure this out as well :-). I think that some things are starting to settle into place. Our current goal is to build out the JavaScript implementation of MyST in order to see how difficult it is and what parts of the Jupyter Book stack can be replicated with an implementation that is simpler than Docutils/Sphinx, and integrates more easily with Jupyter. We'd then hope to maintain both the Python and the JavaScript implementations, but define MyST as a community-governed standard that can be re-used across many projects and implementations.

There are some ideas about this at the end of a recent talk I gave on Jupyter Book at the NumFocus conference:

I think what we should do is open up a discussion about some of these ideas in order to invite community feedback...thus far they haven't been crystallized yet but I think they are starting to take shape. You can also preview some of the JavaScript work in these two docs sites:

And eventually we will need to figure out how these fit in with the project's "meta" documentatoin:

OK I hope that this was some helpful extra context, I am more than happy to discuss with you. I think that making this project more inclusive and adopting healthier asynchronous discussion and development practices is crucial to its success. I want to re-iterate how much I appreciate your thoughtful and helpful comment.

@choldgraf
Copy link
Member

One other note: I think that many of the challenges that you've described are also in-part because we have not done a good job of growing the initial grant-funded team to include broader participation from the community. This is particularly true for roles like stewarding community forums, triaging and guiding issues, etc. I believe this is one direction that the project should prioritize funding that it gets in the future, but I also think we need to grow our volunteer contributor base in this direction as well. I welcome any suggestions you might have for how we can make it easier for others to contribute directly to the project in whatever form they wish.

@kai-tub
Copy link

kai-tub commented Oct 6, 2022

Thank you for your kind words!
I will go through each item individually and add it to the appropriate discussion, but one quick thing I would like to add to the discussion:

Our current goal is to build out the JavaScript implementation of MyST in order to see how difficult it is and what parts of the Jupyter Book stack can be replicated with an implementation that is simpler than Docutils/Sphinx, and integrates more easily with Jupyter. We'd then hope to maintain both the Python and the JavaScript implementations, but define MyST as a community-governed standard that can be re-used across many projects and implementations.

Yes, I am very much aware of the js work from the curvenote team, as I saw some of your discussion in the meta repo, I think.

But this splintering across languages is exactly what makes me worried as a (potential) contributor.
I cannot help but ask myself why there isn't a push toward a Rust (or similar) implementation that could be shared across different languages with FFI bindings (was this the term?).
If I contribute to the Python part, how is it shared with the JS implementation? What will be the default?
Especially the parser and the AST implementation could be provided as a 'golden' standard.

I do not have any experience with actually packaging and distributing Rust within Python/JS, but it seems like more and more projects (polar for example) utilize this approach. And there is a lot of advertisement about WASM (I know that this has been advertised for years now), and for an outside observer, it seems like the trend might be converging towards WASM+Rust for quite a few projects. I am heavily biased though, as I am very interested in this direction and am not a friend of JS.

I understand the heavy connection towards the Jupyter project and their (totally obvious) link to JS, especially because they are doing so much in the Frontend and are working with the DOM.
(Off-topic: Though it is always interesting to see how many Python users are using Jupyter but have trouble contributing because so much is in JS.)


Ok, I just looked at the mystjs page and it has changed a lot since I last looked at it.
I think I better understand in which direction the project is moving.
It is quite impressive but I guess just more underlines the complex project structure and possible co-dependencies or diverging implementations.

I will never be a fan of JS, and I will never be objective when it comes to this language.
I think it is important to 'check' Sphinx/Docutils alternatives but it also makes it a bit weird for people to 'follow' the project coming from Python.
Which is, I assume, still the biggest Jupyter users community and the biggest Sphinx/Docutils community.

I guess you will hear from me again at some point 👍

Cheers

@kai-tub
Copy link

kai-tub commented Oct 6, 2022

This [community page] is an interesting idea - you think it'd make more sense to delegate this to each sub-project rather than have one centralized place for it?

Let's assume that the project is added to a community page on MyST-NB.
I feel like it should belong to this 'documentation' website because it is specific to MyST-NB and doesn't require the full jupyter-book library.
But I assume that most users that would benefit from it would come from the jupyter-book community and not from the MyST-NB only users.
But then the community page would have to be duplicated (especially since this requires changes to the set-up).
As an external contributer, I would be hesitant to ask at two different projects to be included on two community pages.
Maybe there is a way to "inherit" community pages from the lower stack and adding some comments to it?

I thought about this some more, and I think that I don't have a strong preference.
On the one hand, I, as a consumer, would like to have everything in one place to see the ecosystem and popularity of the project + that would be the place I would go to if I wanted to look for alternatives/extensions.
A developer who (potentially) just wants to target MyST-NB and not the jupyter-book community might prefer a different page for each project?
But I guess if somebody targeted MyST-NB, they would be open to also helping JupyterBook users?

The single page just has to be discoverable for users that are looking at the MyST-NB documentation, for example.
At least, that would be my main concern for the single-page solution.

Feel free to move this to a separate issue/discussion.

@chrisjsewell
Copy link
Member Author

thanks for the feedback @kai-tub

I think I better understand in which direction the project is moving.
If I contribute to the Python part, how is it shared with the JS implementation? What will be the default?
Especially the parser and the AST implementation could be provided as a 'golden' standard.

I'd note that I have created https://github.com/executablebooks/myst-eps, precisely for this underpinning design and strategy documentation.
This is exactly the place where such a gold standard would be solidified

@kai-tub
Copy link

kai-tub commented Oct 7, 2022

👍

Yes, I agree.
I guess I didn't really make the mental connection that the Python and JS ecosystems still fall under the MyST project.
Thanks for pointing that out :)

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

3 participants