-
Notifications
You must be signed in to change notification settings - Fork 155
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
mitosheet: handle unset colors in conditional formatting #1279
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -80,6 +80,7 @@ const SetDataframeFormatTaskpane = (props: SetDataframeFormatTaskpaneProps): JSX | |||
} | |||
|
|||
const updateDataframeFormatParams = (newParams: RecursivePartial<DataframeFormat>): void => { | |||
console.log(newParams) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this approach could be more clean and maintainable, unless I'm misunderstanding something.
// If the user does not change the color or background color, we want to treat them as undefined | ||
// so it doesn't change the data in the Mito Spreadsheet or the created Styler or Excel file | ||
newParams.conditional_formats = newParams?.conditional_formats?.map(conditionalFormat => { | ||
const newConditionalFormat = {...conditionalFormat}; | ||
if (newConditionalFormat.color === "var(--mito-text)") { | ||
newConditionalFormat.color = undefined; | ||
} | ||
if (newConditionalFormat.backgroundColor === "var(--mito-background-off)") { | ||
newConditionalFormat.backgroundColor = undefined; | ||
} | ||
return newConditionalFormat; | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a weird way to do this. Why not just default them to undefined when we create them?
This actually isn't a solution |
Description
Fixes #1274
Testing
Please provide a list of the ways you can "access" or use the functionality. Please try and be exhaustive here, and make sure that you test everything you list.
Documentation
Note if any new documentation needs to addressed or reviewed.