Skip to content

Use variables for all colors#276

Merged
severo merged 7 commits intomasterfrom
use-variables-for-all-colors
Sep 23, 2025
Merged

Use variables for all colors#276
severo merged 7 commits intomasterfrom
use-variables-for-all-colors

Conversation

@severo
Copy link
Copy Markdown
Contributor

@severo severo commented Sep 23, 2025

No breaking change.

Use variables for all colors, to make it easier to change the theme colors. I will help in particular, to implement a dark theme (asked here hyparam/hyparquet#114 and here: source-cooperative/parquet-table#4).

We might want to reduce the number of colors and harmonize as a follow-up.


Also, fixes the color of the column menu items (removes the need to fix it, as in https://github.com/hyparam/demos/blob/2a45a13b3294613bf9f97718434baf87624becdb/hyparquet/src/index.css#L320-L322, when it's overridden by the default button color)

@severo severo requested a review from Copilot September 23, 2025 10:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors hardcoded color values throughout the CSS to use CSS custom properties (variables), improving maintainability and theming capabilities. The change also moves the CSS module import path and removes unused animation keyframes.

  • Introduces comprehensive CSS color variables organized by text, background, and border colors
  • Replaces all hardcoded color values with semantic variable references
  • Consolidates color definitions to enable easier theme customization

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/components/HighTable/HighTable.tsx Updates CSS module import path to reference moved stylesheet
src/HighTable.module.css Defines color variables and replaces hardcoded colors throughout, removes unused shimmer animation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/HighTable.module.css Outdated
Comment thread src/HighTable.module.css Outdated
Comment thread src/HighTable.module.css Outdated
Comment thread src/HighTable.module.css Outdated
@severo severo requested a review from Copilot September 23, 2025 10:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/components/HighTable/HighTable.tsx
@severo
Copy link
Copy Markdown
Contributor Author

severo commented Sep 23, 2025

fyi @platypii. I didn't ask for a review because it does not break anything.

Note that it makes very clear that we use (too?) many colors. What about reducing it in the default theme.

@severo severo closed this Sep 23, 2025
@severo severo reopened this Sep 23, 2025
@severo severo merged commit 03897b1 into master Sep 23, 2025
15 checks passed
@severo severo deleted the use-variables-for-all-colors branch September 23, 2025 10:31
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.

2 participants