Skip to content

Conversation

@Lehoczky
Copy link
Contributor

Just a simple fix for the lightbulb's tooltip on the documentation site:

Before:
image
image

After:
image
image

@Lehoczky
Copy link
Contributor Author

Oh I see that https://fastapi.tiangolo.com/ uses the same tooltips. Should I create a PR there too, or is this intentional and not a mistake?

@ArcLightSlavik
Copy link

Looks similar if not the same? fastapi/fastapi#3246

@Lehoczky
Copy link
Contributor Author

Maybe swapping the icons would be better too, sure. While using the light theme the bulb would be "turned on" and with the dark theme it would be "off". Makes sense to me.

However fastapi/fastapi#3246 does not fix the tooltip texts.

@Lehoczky
Copy link
Contributor Author

Anyway, swapping the text alone seems like an improvement for me. Swapping the icons can be something for another PR.

@codecov
Copy link

codecov bot commented Dec 3, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@ed0d670). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main       #1   +/-   ##
=======================================
  Coverage        ?   97.27%           
=======================================
  Files           ?      175           
  Lines           ?     5842           
  Branches        ?        0           
=======================================
  Hits            ?     5683           
  Misses          ?      159           
  Partials        ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed0d670...bce07dc. Read the comment docs.

@tiangolo
Copy link
Member

tiangolo commented Dec 3, 2021

Great, thank you @Lehoczky! 🚀

And thanks for the comments @ArcLightSlavik!

@tiangolo tiangolo changed the title 🐛Fix tooltip text for theme switcher 🐛Fix docs light/dark theme switcher Dec 3, 2021
@tiangolo tiangolo merged commit 82935ca into fastapi:main Dec 3, 2021
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

#1 fix

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.

3 participants