Skip to content

fix: style panel not detecting applied text color (shows "None") #1813

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Nithishvb
Copy link
Contributor

@Nithishvb Nithishvb commented Apr 25, 2025

Description

This PR addresses an issue where the style panel failed to detect and display the applied text color on elements.

Related Issues

Closes #1762

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Release
  • Refactor
  • Other (please describe):

Testing

Screenshots (if applicable)

Additional Notes


Important

Fixes bug in ColorInput by resolving CSS variables to correctly detect applied text color.

  • Behavior:
    • Fixes bug in ColorInput where applied text color was not detected due to unresolved CSS variables.
    • Uses resolveCssVariables to resolve CSS variables in ColorInput.
  • Functions:
    • Adds resolveCssVariables to color.ts to replace CSS variables with actual values from styleRecord.

This description was created by Ellipsis for 9d4b4ed. You can customize this summary. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 9d4b4ed in 1 minute and 17 seconds. Click for details.
  • Reviewed 50 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. apps/studio/src/routes/editor/EditPanel/StylesTab/single/ColorInput/index.tsx:101
  • Draft comment:
    Consider using the resolved value for theme lookup instead of the original newValue. This avoids inconsistency when CSS variables are used.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. packages/utility/src/color.ts:342
  • Draft comment:
    The fallback value '1' may not be an ideal default color value when a CSS variable is missing. Consider using a more conventional fallback (e.g., a valid color or transparent) for clarity.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
3. apps/studio/src/routes/editor/EditPanel/StylesTab/single/ColorInput/index.tsx:106
  • Draft comment:
    Consider using 'resolvedValue' instead of 'newValue' when calling getColorByName so that CSS variables are resolved before theme lookup.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
4. packages/utility/src/color.ts:342
  • Draft comment:
    The regex in resolveCssVariables does not account for whitespace inside var() calls; also, verify if using a fallback of '1' (which yields 'Incremented version v0.0.7 #111') is appropriate.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None

Workflow ID: wflow_sHtMoar6cHYYv3UQ

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

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.

[bug] Style panel does not detect applied text color (shows "None")
1 participant