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

Full refactor of napari-sphinx-theme to depend on pydata-sphinx-theme #134

Merged
merged 8 commits into from
Jan 16, 2024

Conversation

melissawm
Copy link
Member

@melissawm melissawm commented Nov 7, 2023

Refactors colors, styling and fonts from napari-sphinx-theme. Also removes extra react components that are not needed for the napari theme.

Includes the community calendar feature, but calendar is now managed on the docs side.

To see how this would look like, you can fetch this PR and build the docs locally (after fetching napari/docs#267), or you can see the deployed site here: https://melissawm.github.io/napari-theme-var

Important

After merging, this will also require a new release of the napari-sphinx-theme.

Addresses #113

See also napari/docs#267

Refactors colors, styling and fonts from napari-sphinx-theme. Also
removes extra react components that are not needed for the napari theme.

Includes the community calendar feature, but calendar is now managed on
the docs side.
pyproject.toml Outdated Show resolved Hide resolved
Copy link
Member

@jni jni left a comment

Choose a reason for hiding this comment

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

❤️❤️❤️

I'll approve, but wait a couple of days for comments. Thank you @melissawm!!!

docs/conf.py Show resolved Hide resolved
@@ -0,0 +1,67 @@
<svg
Copy link
Member

Choose a reason for hiding this comment

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

What is this file? Is it a tiny svg logo? If so, could it be renamed to indicate that?

Copy link
Member Author

Choose a reason for hiding this comment

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

They are (this and all the other partials too). Because the theme expects templates here, it won't easily accept svg files but it will accept an html. To change that would imply a more complicated change of the footer template. I can take a look if there's no rush here.

Copy link
Member

@psobolewskiPhD psobolewskiPhD left a comment

Choose a reason for hiding this comment

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

We should update the README too!

Marking request changes based on discussion at Docs working group to hold off with merge until 0.4.19 is released and the calendar events are more presentable, versus current:
image

One option is to edit them on the side of the google calendar, see:
https://scientific-python.org/calendars/
image

Not as fancy as the current napari theme, but much more legible than the new theme.

@melissawm
Copy link
Member Author

I think I addressed all comments except for the calendar - let me know if there's anything else to do here. Cheers!

@jni
Copy link
Member

jni commented Dec 28, 2023

Hi folks,

After some discussion with @Czaki, I don't think we need to wait until 0.4.19 to merge and release this, since our docs builds pin the napari-sphinx-theme version. I would like to merge this and release the theme, which will unlock the PRs on the docs side. But @melissawm I'm unclear about where the calendar pop-up needs to be fixed — is it here in the theme, or is it in the docs repo?

@jni
Copy link
Member

jni commented Dec 28, 2023

(But I'm more than happy to clean up the events to be plaintext, too.)

@psobolewskiPhD
Copy link
Member

My understanding is it's the actual events in google? that need fixing.

@melissawm
Copy link
Member Author

If the events are fixed on the calendar side, it will both make them more readable right now and more easily formatted into a nice pop-up later, so that's helpful. I can also prioritize this today to bring the napari-docs PR up to date. I'll let you know. Thanks! 🚀

@melissawm
Copy link
Member Author

Ok! Just rebuilt everything and rebased the napari-docs PR. This should be good to go except for the calendar as mentioned before. Also - for this to be active on the website, we need to cut a new release of this theme.

@psobolewskiPhD
Copy link
Member

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented Jan 15, 2024

I checked the calendar:
https://melissawm.github.io/napari-theme-var/community/meeting_schedule.html
This should be using the latest of everything right?
Things look a bit better?
image

I think it will either look bad in the new theme or look bad on google calendar?

@jni
Copy link
Member

jni commented Jan 16, 2024

I think it will either look bad in the new theme or look bad on google calendar?

I think Google Calendar is literally adding that html. I edited every event and pasted in plaintext versions of the descriptions, and this is what it looks like? I'm more than a little bit miffed at Google, yet again. I wonder if there are other calendar providers we can use here. (Once again, I wonder if @melissawm has suggestions based on other projects she's seen at Quansight.)

I also noticed when clicking a few events that this is using a javascript prompt, which means FF thinks it's spammy if you click on two events quickly:

Screenshot 2024-01-16 at 7 06 24 pm

So I wonder whether there's another alternative here... Either way, I'm happy to take guidance from @melissawm on whether that should come in a later PR. For now, I think the theme provides enough value that we should merge it and deal with the issues later. But I don't have a great idea what "deal with the issues" actually entails... 😅

@melissawm
Copy link
Member Author

@jni I went with a different modal (not an alert) and I think that does it. Also it looks much better now, since I can really format things properly! Here it is: https://melissawm.github.io/napari-theme-var/community/meeting_schedule.html

Screenshot just in case:
Screenshot_20240116_193136

What do you think?

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented Jan 16, 2024

@jni I went with a different modal (not an alert) and I think that does it. Also it looks much better now, since I can really format things properly! Here it is: https://melissawm.github.io/napari-theme-var/community/meeting_schedule.html

OMG it's perfect!
I have to be honest, the clickable links are 💯 because this is how I access these things!

Copy link
Member

@psobolewskiPhD psobolewskiPhD left a comment

Choose a reason for hiding this comment

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

Approving!
Could you just document/comment on the pydata-sphinx pinning?

@melissawm
Copy link
Member Author

melissawm commented Jan 16, 2024

Yes - this should be a temporary thing. Version 1.15.1 introduces a bug where the left side bars don't show up. This is fixed in main but not yet released. Here are the details: pydata/pydata-sphinx-theme#1625

@jni jni merged commit 4f28aa4 into napari:main Jan 16, 2024
@jni
Copy link
Member

jni commented Jan 16, 2024

🚀🚀🚀🚀 I am elated over here!!!! 😭 🙏🙏🙏🙏

@melissawm could you confirm for me the next steps needed? By my understanding:

@jni
Copy link
Member

jni commented Jan 16, 2024

btw, this thing:

image

😍😍😍😍

@melissawm
Copy link
Member Author

🚀🚀🚀🚀 I am elated over here!!!! 😭 🙏🙏🙏🙏

@melissawm could you confirm for me the next steps needed? By my understanding:

That is also my understanding! I'll be around looking to see if we have any unexpected failures but I think we should be good! 😍 Woohoo!!

@jni
Copy link
Member

jni commented Jan 16, 2024

Ok, there will be a constraints-update required and I think that has to happen on the napari/napari repo. But the build can't work until napari/docs#267 is merged. HMMM. I guess there is a minimum of one broken build in our future, but I think that's ok... Anyway, since the constraints are used when building docs, it is risk-free to release this so imma do it!!!

@melissawm
Copy link
Member Author

I'll update the requirements files on the PR at the docs repo but I'm not really sure how to update the constraints files, so I'll leave those up to you folks 🙏

@jni
Copy link
Member

jni commented Jan 17, 2024

Ok, lots to fix in our release action 😂 hold please 😂

jni pushed a commit to napari/napari that referenced this pull request Jan 17, 2024
# References and relevant issues
Related to napari/napari-sphinx-theme#134,
napari/docs#267

# Description
Updates docs constraints file for the new napari-sphinx-theme release.
jni pushed a commit to napari/docs that referenced this pull request Jan 18, 2024
Prepares the docs repo to use the new napari-sphinx-theme, 0.3.0, which depends on pydata-sphinx-theme rather than forking it.

# References and relevant issues

Addresses napari/napari-sphinx-theme#113
Depends on napari/napari-sphinx-theme#134
Czaki pushed a commit to napari/napari that referenced this pull request Jan 21, 2024
Prepares the docs repo to use the new napari-sphinx-theme, 0.3.0, which depends on pydata-sphinx-theme rather than forking it.

# References and relevant issues

Addresses napari/napari-sphinx-theme#113
Depends on napari/napari-sphinx-theme#134
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