-
Notifications
You must be signed in to change notification settings - Fork 62
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
🔧 MAINTAIN: Drop furo-specific stylesheet #22
Conversation
Codecov Report
@@ Coverage Diff @@
## main #22 +/- ##
=======================================
Coverage 88.82% 88.82%
=======================================
Files 10 10
Lines 859 859
=======================================
Hits 763 763
Misses 96 96
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
Cross post question: pradyunsg/furo#111 (comment) I don’t see this css in furo yet? |
@chrisjsewell I'm unlikely to make the change you've asked for in that comment. As far as I am concerned, this stylesheet is completely wrong, since https://pradyunsg.me/furo/changelog/#beta39. I've overridden everything that this sets in Furo, and if you want sphinx-design to be usable in the current versions of Furo, it'd be best to just straight up drop this stylesheet IMO. I'll defer to others on whether this should be merged or not. I'll note that if this PR gets rejected, I intend to remove the sphinx-design-specific stylesheets from Furo and also remove the relevant sections recommending/advertising support from Furo's documentation. |
I'm a bit surprised by this comment, it feels a bit aggressive, no? Just to clarify, this stylesheet is not part of the sphinx-design package, it is only used for the internal documentation, i.e. there is nothing for Furo to override unless users specifically choose to add this CSS to their package (outside of using the sphinx-design extension) This is not to say I'm against removing it. I would have imagined the more critical issue would be #21, which I was still hoping for a reply from you 😬 |
Apologies for this. This is frustration from elsewhere creeping into the wrong place. I spent the whole day fighting the browser and some other drama -- triaging my own open PRs after that wasn't the best plan. 😓
Indeed.
Yep, I'll update that today. :) |
I still do suggest dropping this stylesheet entirely -- having this means that the users don't get a representative view of how the extension will behave in their documentation, which... isn't ideal. :) |
Ah yeh no worries, we've all been there lol. I'll try to get this sorted soon 👍 |
Ok so: As it is now; after #21, with furo==2021.7.5b38, and with the stylesheet with furo==2021.7.5b38, but no stylesheet With furo==2021.10.09, no stylesheet |
So are you going to keep these new overrides, which take it away from the look of #21 (comment) 😬 |
You're missing an important detail: there's a dark mode toggle in Furo. :) |
but what does that change? |
With dark mode on your browser, click the moon-like button on the top of the content a few times. It'll be broken when the on-page toggle gets to light mode. Likewise for the other way around. I'll tweak things over on Furo's end once this makes it into a release, and I can test it easily. :) |
Now released BTW. Have a play around and then let me know 👍 |
What's the blockers to merging this again? |
will circle back around to this soon 😬 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cheers @pradyunsg, sorry this fell off my radar a bit!
No worries, cheers! :) |
Furo is gonna add styles for supporting this extension directly. This can be merged once I make a Furo release with the relevant changes.