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

Themes and command palette improvements #5087

Merged
merged 174 commits into from
Nov 13, 2024
Merged

Themes and command palette improvements #5087

merged 174 commits into from
Nov 13, 2024

Conversation

darrenburns
Copy link
Member

@darrenburns darrenburns commented Oct 3, 2024

  • Removes App.dark
  • Adds App.theme: Reactive[str] which can be used to set the theme (registered themes only).
  • Adds variant parameter to Label (success/primary/accent/error, etc.)
  • Adds InvalidThemeError, raised when an unregistered theme is set.
  • Adds App.get_theme(theme: str) -> Theme
  • Adds App.register_theme(theme: Theme)
  • Adds App.unregister_theme(theme: Theme)
  • Adds App.theme_changed_signal: Signal[Theme].
  • Adds App.available_themes -> dict[str, Theme]
  • Fix Select widget sometimes immediately re-opening when closed with a click of the mouse
  • Allows CommandPalette to be instantiated and pushed by users, with arbitrary providers and a custom placeholder per-instance.
  • Added SimpleProvider which allows a caller to pass commands into a CommandPalette rather than having a provider be aware of where it might be invoked (experiment).
  • Added App.search, a shorthand for the above that lets you quickly invoke a searchable menu with a list of commands, e.g. self.app.search([("copy", self.copy), ("paste", self.paste)] (an experiment - creating a separate provider each time feels like a big overhead and the providers need to query the active screen etc.)
  • Adds App.search_themes which pulls up a command palette containing all registered themes. This is what is called when you select "Change theme" in the command palette.
  • Defines some new builtin CSS vars $hover, $cursor-highlight, $cursor-highlight-blurred, $hover-highlight, $hover-highlight-blurred.
  • Tries to make usage of variables a bit more consistent without changing the look and feel of Textual.
  • Changes tree and data table cursors to match other cursors, and removes the "green" accent from the tree lines.
  • Updates a bunch of widgets to use nested CSS instead of flat CSS as it seems easier to read and keep related rules together.
  • I tried to make RadioSet look a bit more like SelectionList - haven't quit achieved what I intended there though.
  • All widgets with a "highlight cursor" (the blue rectangle behind the highlighted option) now have a "blurred" version of that cursor, so when the widget loses focus, the cursor fades slightly.
  • Some widgets had an underline indicating the "highlighted" option (e.g. RadioSet) - I replaced this with a cursor (just changing the styles). It's more coherent/consistent with other widgets, and also gains the "blurred" cursor effect when it loses focus. I'm not sure it looks better in isolation 🤷
  • Added consistent block cursor styling.
textual-themes-3oct24__89pct_smaller.mp4

Widget borders

I tried removing the widget borders and although I do think they look better in isolation, when they touch each other there's no way to distinguish between them:

image

I opted to keep the border but make it very subtle:

image

Background tint

The majority of builtin widgets now get a subtle background tint when focused e.g:

image

This is particularly useful for users who prefer compact styles and remove borders and therefore don't receive the "focus styling" that gets applied to borders.

@darrenburns darrenburns changed the title Themes Themes and command palette improvements Oct 7, 2024
src/textual/design.py Outdated Show resolved Hide resolved
@darrenburns darrenburns marked this pull request as ready for review November 12, 2024 15:43
### Dark mode
## Light and dark themes

Themes can be either "light" or "dark".
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the quotes may confuse there. It suggests that they are strings.

Maybe something along these lines?

Themes may be marked as being light or dark.

Copy link
Collaborator

Choose a reason for hiding this comment

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

😢

@@ -698,17 +703,17 @@ class Markdown(Widget):
| :- | :- |
| `code_inline` | Target text that is styled as inline code. |
| `em` | Target text that is emphasized inline. |
| `s` | Target text that is styled inline with strykethrough. |
| `s` | Target text that is styled inline with strikethrough. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

I kinda liked the original spelling better.

@willmcgugan willmcgugan merged commit 5de3a80 into main Nov 13, 2024
20 checks passed
@willmcgugan willmcgugan deleted the themes branch November 13, 2024 10:30
@TomJGooding
Copy link
Contributor

TomJGooding commented Nov 13, 2024

I spotted that the unselected crosses in the SelectionList aren't visible in the textual-light theme, was this intentional? I just thought I'd check as the similar widgets look okay.

I'm also finding it a bit difficult to make out the scrollbar on the screen in light mode, but maybe that's just to my eyes! Especially in 256-color terminals where the thumb barely distinguishable . Hope you don't mind my late feedback, I realise this has already been merged...

image

@darrenburns
Copy link
Member Author

Thanks Tom, we're doing another pass now before releasing this. Tweaking scrollbars now, but didn't notice the issue with the SelectionList - will get that sorted too. Cheers!

@TomJGooding
Copy link
Contributor

Sorry I've also just spotted there's a similar issue in the RadioSet when focused (the scrollbar looks better now!)

image

@darrenburns
Copy link
Member Author

@TomJGooding cheers - working on SelectionList, RadioSet and ToggleButton now! Will try to make them consistent on light themes.

@TomJGooding
Copy link
Contributor

Sorry will leave this now in your capable hands! This is a fantastic feature and I can see the work that's gone into it, hopefully you don't think I'm just nitpicking.

@darrenburns
Copy link
Member Author

No no, not at all! We appreciate it. This was so much ground to cover and I definitely missed a few things. If you find anything else that you think looks off please let us know :)

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