[GitHub] [superset] rusackas commented on pull request #22011: fix: Change downloadAsImage to use Superset theme

[GitBox <gi...@apache.org>]

rusackas commented on PR #22011:
URL: https://github.com/apache/superset/pull/22011#issuecomment-1301464213

   I applaud your effort in reinforcing proper theme usage! I also randomly enjoyed a little trip down the commit history that this led to! 
   
   Regarding the comment you refer to in the code and its link to line 34, this gets interesting. At the time of [the PR adding that comment](https://github.com/apache/superset/pull/9819) to the codebase, the referenced line 34 actually pointed to a value that was indeed `#f5f5f5` as the variable added alludes to. Checking your screenshots, it looks like `before` and `after` are both the same color here (slightly blue) but I _think_ the `before` screenshot would/should be `#f5f5f5`, right? I think that rather than changing this value to one based on `primary` (which might be any random color when we get into theming) I would go for the closest supported grayscale color. In the theme, that looks like `grascale.light4` or `#f7f7f7`.
   
   Let me know if that all makes sense :) 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org