Skip to content

Conversation

@bettina-gier
Copy link
Contributor

@bettina-gier bettina-gier commented May 22, 2025

Changes in this PR:

  • Updated gallery layout
  • Added an option to choose which plot(s) will appear in the gallery

Also includes small fixes:

  • Several fixes to wrong figure captions in recipe documentations
  • Moved Legacy recipe documentation in a legacy folder
  • Removed the example recipe that was linked from the IPCC subsection

@bettina-gier bettina-gier requested a review from axel-lauer May 23, 2025 09:32
@bettina-gier bettina-gier marked this pull request as ready for review May 23, 2025 09:33
@valeriupredoi
Copy link
Contributor

many thanks, Tina! As Borat once said, I like! 🍻

@lukruh
Copy link
Contributor

lukruh commented May 23, 2025

Thanks for starting this. I generally like the approach. But falling back to the first plot, when multiple plots use gallery=True sounds bad. When we add an option to toggle visibility in the gallery, it should work for multiple plots per recipe documentation and it also should always hide plots when explicitly set to false.

When no gallery-options are given in the file, I think it is fine to fall back to the first one (depending on how easy it is to implement). This adds a bit of complexity but keeps it consistent with the current behavior.

@lukruh
Copy link
Contributor

lukruh commented May 27, 2025

I have had a closer look to the gallery generating script. instead of gallery=True/False I'd suggest to use simple flags like .. gallery and .. no-gallery. I also found some more minor issues that i think could be improved regarding the gallery:

  1. The parsing seems a bit fragile. Like searching for strings or get the label from the first row etc. This can be improved (and also simplify selecting multiple figures) by using sphinx's docutils to parse the rst files instead of doing it manually.
  2. The table layout for the figures is not really beutiful (in my case the columns are not even the same witdth).
  3. Figure captions should be close to the figure. Jumping to the bottom of a page to read a caption or follow the recipe link feels a bit complicated.

Unfortunatly it cannot be easily fixed by "per pager template" or "html input" (not well supported out of the box). But we can still improve the layout with some limitations using raw html directive in rst. However, I added a commit with some ideas how this could be changed. It's just meant to be an example for the discussion not the final code. Feel free to revert it if you don't like it. If we agree on doing it this way, I can add some css to make it look a bit cleaner.

@lukruh
Copy link
Contributor

lukruh commented May 28, 2025

We added a little bit of CSS to make the layout more responsive and usable (and prettier IMO). @bettina-gier also spotted some recipes with formatting issues and/or missing captions (thanks for fixing them).

Have a look at the updated gallery: https://esmvaltool--4060.org.readthedocs.build/en/4060/gallery.html

@lukruh lukruh changed the title Change Gallery to skip Legacy recipes but include subfolders Update figure selection and layout of the gallery May 28, 2025
@bouweandela
Copy link
Member

Removed the example recipe that was linked from the IPCC subsection (WTH?)

One of the example recipes reproduces an IPCC figure: examples/recipe_easy_ipcc.yml. Therefore it made sense to me to list that under the IPCC recipes. Happy for you to have a different opinion of course

@bettina-gier
Copy link
Contributor Author

Removed the example recipe that was linked from the IPCC subsection (WTH?)

One of the example recipes reproduces an IPCC figure: examples/recipe_easy_ipcc.yml. Therefore it made sense to me to list that under the IPCC recipes. Happy for you to have a different opinion of course

In theory that's fine, but then maybe we should subsection the example recipes and can link directly to it? Same way it now looks with the droughts and legacy recipes - have a folder for example recipes? The way it was set up with the same link in the list twice I didn't really like and felt intuitive. If it's "IPCC example recipe" it would make more sense?

Copy link
Contributor

@schlunma schlunma left a comment

Choose a reason for hiding this comment

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

Thanks @bettina-gier and @lukruh, that's great, I like it!

Since I am not very familiar with HTML/CSS, I cannot really comment on the code itself, but the generated gallery and the changes to the recipes page look very good. I had a look at all pages and tested the links, everything works as expected.

One tiny comment: Would it make sense to document the new gallery feature somewhere?

Thank you 🚀

@bettina-gier
Copy link
Contributor Author

I was looking where to document but I'm not sure. The current "contributing" is quite general and it seems a bit technical to put it there. But maybe as a tip?

@schlunma
Copy link
Contributor

schlunma commented Jun 5, 2025

Somewhere here maybe?

@bettina-gier
Copy link
Contributor Author

I was thinking more here ?

@schlunma
Copy link
Contributor

schlunma commented Jun 5, 2025

Yeah, probably better. To be honest I am not quite sure I understand why we have those two different pages about that 😅

@bettina-gier
Copy link
Contributor Author

added documentation for the tags here

@lukruh
Copy link
Contributor

lukruh commented Jun 17, 2025

@flicj191, looks like you are working on a bigger update of the documentation right now and we might need to merge the changes anyway. Would you have time to review this PR? I could also go trough #3914, in case you are looking for reviewers.

@lukruh lukruh requested a review from flicj191 June 17, 2025 12:16
Copy link
Contributor

@flicj191 flicj191 left a comment

Choose a reason for hiding this comment

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

Thanks @lukruh and @bettina-gier this looks good, I like the new gallery!
Thanks for the heads up @lukruh on the changes, should be straightforward to merge as I have only looked at the navigation headers and landing page.

@lukruh
Copy link
Contributor

lukruh commented Jun 19, 2025

I adjusted the breakpoints to show only 2 or 1 column depending on the available space. If you want to test it resize the brwoser window and make sure the old css is not cached (remove cache or open it in a private window).
Looks like this can be merged now.

Thanks a lot for the helpful reviews.

@bettina-gier bettina-gier merged commit 3b41609 into main Jun 19, 2025
8 checks passed
@bettina-gier bettina-gier deleted the gallery-updates branch June 19, 2025 11:28
@valeriupredoi
Copy link
Contributor

very many thanks for getting this one through, folks 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants