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

Resolve issue with table_font_color not accepting named colors #285

Merged
merged 9 commits into from
Apr 16, 2024

Conversation

jrycw
Copy link
Contributor

@jrycw jrycw commented Apr 13, 2024

It appears we're having an issue with the Table Theme Options -> Set options across table parts section.

image

The problem seems to arise when color values are input using common names like yellow or white, causing the font_color not to transform them into the appropriate hexadecimal format.

To address this issue, I've implemented a conversion pattern similar to the one used in font_color, which should hopefully resolve the issue.

Additionally, I noticed quick returns in the first few lines of font_color. I'm uncertain whether these also need conversion. Could the team please review this aspect?

@codecov-commenter
Copy link

codecov-commenter commented Apr 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.71%. Comparing base (479eb73) to head (7008372).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #285   +/-   ##
=======================================
  Coverage   81.71%   81.71%           
=======================================
  Files          41       41           
  Lines        4325     4327    +2     
=======================================
+ Hits         3534     3536    +2     
  Misses        791      791           

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

@jrycw jrycw changed the title Resolve issue with table_font_color not accepting named Colors Resolve issue with table_font_color not accepting named colors Apr 13, 2024
@machow machow self-requested a review April 16, 2024 17:48
@machow
Copy link
Collaborator

machow commented Apr 16, 2024

Thanks so much for this work! I'm pairing right now with @rich-iannone, and he mentioned this function should always return hex (which it wasn't even before this PR). I've tweaked it a bit, so that when things like "transparent" are passed, hex colors are output.

Copy link
Collaborator

@machow machow left a comment

Choose a reason for hiding this comment

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

LGTM--thanks for catching this issue

@machow machow merged commit 9bebd91 into posit-dev:main Apr 16, 2024
9 checks passed
@jrycw
Copy link
Contributor Author

jrycw commented Apr 17, 2024

The refactored code looks better. In fact, the variable names definitely represent their meaning clearly.

@jrycw jrycw deleted the update-font_color branch April 22, 2024 14:03
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.

None yet

3 participants