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

Add Design and Theme explanation documentation #4741

Merged
merged 2 commits into from
Jan 16, 2024
Merged

Add Design and Theme explanation documentation #4741

merged 2 commits into from
Jan 16, 2024

Conversation

philippjfr
Copy link
Member

@MarcSkovMadsen This is definitely far from perfect but I'm hoping it'll give you an idea of how to contribute to improve the Fast designs (and restore some of the styling that didn't get ported).

@codecov
Copy link

codecov bot commented Apr 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b0b94ad) 82.69% compared to head (7fa76ac) 73.21%.
Report is 1 commits behind head on main.

❗ Current head 7fa76ac differs from pull request most recent head a3b3037. Consider uploading reports for the commit a3b3037 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4741      +/-   ##
==========================================
- Coverage   82.69%   73.21%   -9.48%     
==========================================
  Files         301      268      -33     
  Lines       45099    37681    -7418     
==========================================
- Hits        37293    27589    -9704     
- Misses       7806    10092    +2286     
Flag Coverage Δ
ui-tests ?
unitexamples-tests 73.21% <ø> (+1.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

# Designs and Theming

:::{versionadded} 1.0.0
Designs and theming allow controlling the look and feel of individual components or your entire application.
Copy link
Collaborator

@MarcSkovMadsen MarcSkovMadsen Apr 29, 2023

Choose a reason for hiding this comment

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

Maybe connect to template too. For me I think I want to correct a template, so I'm looking for an explanation for that.

So I probably need to be taught that the starting point is a design. A design can have different themes. And a template can use the design (and theme).

Maybe even visualize how these things are connect via a graph. Or via an a screenshot of an app wrapped in a nice template that uses some design.

All these design stylesheets open with CSS variable definitions which map the global color definitions to specific variables used by a particular design system:

```css
:host, :root {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You will need to explain what :host and :root refer to.

@MarcSkovMadsen
Copy link
Collaborator

Its a good starting point for me. I added my questions and comments. I hope we can use that to improve the PR.

@philippjfr
Copy link
Member Author

Indeed, only a start, all good points in your review.

@MarcSkovMadsen
Copy link
Collaborator

MarcSkovMadsen commented Apr 30, 2023

One thing I need to do is to change for the fast templates is the border-left-color below to var(--accent-foreground-active). I've been trying for an hour now, and I simply don't know how to do this. How can I do this @philippjfr , @mattpap ?

image

What I've been trying is to add some selector to the file https://github.com/holoviz/panel/blob/main/panel/template/fast/fast.css

For example

div.bk-right.bk-active {
    border-left-color: yellow;
}
:root, :host {
    --active-tool-highlight: green !important;
}
:host(.bk-right.bk-active){
    --active-tool-highlight: salmon !important;
}
.bk-right.bk-active {
    --active-tool-highlight: pink !important;
}

The only selector that seems to get near is :host. But not enough.

image

Context

Pre Panel 1.0/ Bokeh 3.0 we could change the tool button border and the tooltips color via the this css. I would like to achieve the same effects for Panel 1.0 to beautify the Bokeh plots.

@philippjfr
Copy link
Member Author

Any component in the shadow DOM cannot be styled from the template CSS file. As laid out in the document those rules have to go in panel/theme/css/fast.css

@mattpap
Copy link
Collaborator

mattpap commented Apr 30, 2023

--active-tool-highlight is a local CSS variable, thus the only way to override is to add a stylesheet to the component (or a class of components). To theme a component (i.e. to override CSS variables from global scope), component's CSS variables have to be set up like this:

--active-tool-highlight: var(--theme-active-tool-highlight, #26aae1);

where theme suffix is discretionary (i.e. whatever as long as there's distinction between local and global variables). The global variable by default is unset, which makes the local variable to use the literal default value. Note you can chain multiple CSS variables for more advanced themes. Most bokehjs' CSS variables are local, and only a few so far as set for theming. To override the local variable in a theme, one would use:

:root {
  --theme-active-tool-highlight: red;  /* or var(--another-themed-variable) */
}

Note that :root pseudo selector only works in the global scope and usually is equivalenthtml selector, just has higher specificity (pretty useful to avoid !important in some cases). :host pseudo selector only works in shadow DOM and refers to the shadow root element. :root and :host should not be used together.

@philippjfr
Copy link
Member Author

@mattpap Thanks for the explanation, unfortunately in this case I can't see how we'd even go about injecting anything into the shadow DOM of the tool button, as far as I can tell the tool button does not have a corresponding Python model that we could inject the stylesheets on.

@MarcSkovMadsen
Copy link
Collaborator

Is it correctly understood that currently we cannot style the tooltip and toolbar buttons?

@philippjfr
Copy link
Member Author

Waiting to hear back from @mattpap but to me it looks like that. The tool buttons are internal models so can't easily be controlled from Python. IMO all tool related CSS variables should therefore be set at the toolbar level not at the tool button level.

@mattpap
Copy link
Collaborator

mattpap commented Apr 30, 2023

The tool buttons are internal models so can't easily be controlled from Python.

Each button is an individual component and then toolbar is another component. Tool buttons are exposed in bokehjs, but not it Python, because I couldn't settle on a stable API before 3.1 release.

@MarcSkovMadsen
Copy link
Collaborator

What would the next step be here @philippjfr ? A feature request to Bokeh?

@philippjfr
Copy link
Member Author

Yes, absolutely. That said I'm not entirely sure what shape that would take. I don't really see how we can expose ToolButton at the Python level or if that is even desirable. I'll let @mattpap suggest an approach but maybe let's start by making an issue that simply outlines the problem which is that the --active-tool-highlight default on ToolButton cannot currently be overridden.

@philippjfr
Copy link
Member Author

I haven't addressed all review comments but this is already much better than nothing.

@philippjfr philippjfr merged commit 9264e9a into main Jan 16, 2024
2 of 3 checks passed
@philippjfr philippjfr deleted the design_docs branch January 16, 2024 12:19
@MarcSkovMadsen
Copy link
Collaborator

MarcSkovMadsen commented Jan 16, 2024

I haven't addressed all review comments but this is already much better than nothing.

I feel the same for the ReactiveHTML docs PR 😄 Maybe it could also be merged?

@philippjfr
Copy link
Member Author

Yes, indeed. Was planning to do that.

philippjfr added a commit that referenced this pull request Jan 17, 2024
* Add Design and Theme explanation documentation

* Address a few review comments
This was referenced Jan 17, 2024
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