Skip to content

Conversation

@Junjiequan
Copy link
Member

@Junjiequan Junjiequan commented Dec 5, 2025

Description

first draft of theme doc

Motivation

Background on use case, changes needed

Fixes:

Please provide a list of the fixes implemented in this PR

  • Items added

Changes:

Please provide a list of the changes implemented by this PR

  • changes made

Tests included

  • Included for each change/fix?
  • Passing? (Merge will not be approved unless this is checked)

Documentation

  • swagger documentation updated [required]
  • official documentation updated [nice-to-have]

official documentation info

If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included

Backend version

  • Does it require a specific version of the backend
  • which version of the backend is required:

Summary by Sourcery

Introduce documentation for the front-end CSS variable–based theme system and newly added theme variables.

Documentation:

  • Add a technical overview of the CSS variable–based theming system used by the SciCat front-end.
  • Document newly introduced theme variables for side panel and table borders and shadows, including defaults, fallbacks, and usage notes.

@Junjiequan Junjiequan requested a review from a team as a code owner December 5, 2025 10:35
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • In the Newly Added Variables section, the default box-shadow values for --theme-side-panel-shadow and --theme-table-shadow end with an extra backtick, which makes the snippets look like malformed CSS; consider removing the trailing backticks or wrapping the whole value in a code block instead.
  • The note that --theme-*-border has a component-level fallback of 1px solid rgba(153, 213, 233, 0.7) conflicts with the earlier statement that the default is none; clarifying the difference between the theme default and the SCSS fallback would make the behavior easier to understand.
  • It might help readers if you add a short example showing where to define these CSS variables (e.g., on :root or a theme-specific class) so it’s clear how to actually apply the documented values in a deployment.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the `Newly Added Variables` section, the default box-shadow values for `--theme-side-panel-shadow` and `--theme-table-shadow` end with an extra backtick, which makes the snippets look like malformed CSS; consider removing the trailing backticks or wrapping the whole value in a code block instead.
- The note that `--theme-*-border` has a component-level fallback of `1px solid rgba(153, 213, 233, 0.7)` conflicts with the earlier statement that the default is `none`; clarifying the difference between the theme default and the SCSS fallback would make the behavior easier to understand.
- It might help readers if you add a short example showing where to define these CSS variables (e.g., on `:root` or a theme-specific class) so it’s clear how to actually apply the documented values in a deployment.

## Individual Comments

### Comment 1
<location> `docs/theme/color-theme.md:28` </location>
<code_context>
+
+`--theme-primary-default-contrast`
+
+… Above variables control general old theme colors (modify as needed)
+
+
</code_context>

<issue_to_address>
**suggestion (typo):** Clarify wording in the description of the existing theme variables.

The sentence "… Above variables control general old theme colors (modify as needed)" is a bit awkward. Consider something like: "The above variables control the general (legacy) theme colors (modify as needed)."

```suggestion
The above variables control the general (legacy) theme colors (modify as needed).
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Junjiequan Junjiequan enabled auto-merge (squash) December 5, 2025 12:58
@Junjiequan Junjiequan merged commit dc54477 into master Dec 5, 2025
7 checks passed
@Junjiequan Junjiequan deleted the theme-doc branch December 5, 2025 13:07
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