You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "EmmanuelCbd (via GitHub)" <gi...@apache.org> on 2025/05/28 09:24:42 UTC
[PR] Adds "info" and "info dark" (blue colors) values to edit formatter color scheme [superset]
EmmanuelCbd opened a new pull request, #33601:
URL: https://github.com/apache/superset/pull/33601
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
This PR adds "info" and "info dark" values to edit formatter color scheme drop down.
It allows users to use blue color for conditions
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
Before:
After:
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [X] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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
Re: [PR] feat(charts): Adds "info" and "info dark" (blue colors) values to edit formatter color scheme [superset]
Posted by "EmmanuelCbd (via GitHub)" <gi...@apache.org>.
EmmanuelCbd commented on PR #33601:
URL: https://github.com/apache/superset/pull/33601#issuecomment-2944467160
@mistercrunch , I was about to suggest using the `active` column, like `colorErrorActive`, but as you said, it would abuse of the semantic.
I'd love to help more on that, but I'm absolutly not familiar with `tinycolor2` and the usage of it.
--
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
Re: [PR] feat(charts): Adds "info" and "info dark" (blue colors) values to edit formatter color scheme [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed pull request #33601: feat(charts): Adds "info" and "info dark" (blue colors) values to edit formatter color scheme
URL: https://github.com/apache/superset/pull/33601
--
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
Re: [PR] feat(charts): Adds "info" and "info dark" (blue colors) values to edit formatter color scheme [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #33601:
URL: https://github.com/apache/superset/pull/33601#issuecomment-2917107454
Aside from @kasiazjc 's input, we should CC @mistercrunch / @geido since this will likely have some sort of impact with theming.
--
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
Re: [PR] feat(charts): Adds "info" and "info dark" (blue colors) values to edit formatter color scheme [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #33601:
URL: https://github.com/apache/superset/pull/33601#issuecomment-2917077691
@EmmanuelCbd The colors follow the defined designed system. We need to check with @kasiazjc if we need additional colors.
--
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
Re: [PR] feat(charts): Adds "info" and "info dark" (blue colors) values to edit formatter color scheme [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #33601:
URL: https://github.com/apache/superset/pull/33601#issuecomment-3882986420
Closing due to contributor inactivity for several months. Feel free to reopen if/when warranted, and thank you for the contribution!
--
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
Re: [PR] feat(charts): Adds "info" and "info dark" (blue colors) values to edit formatter color scheme [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #33601:
URL: https://github.com/apache/superset/pull/33601#issuecomment-2941940538
This will merge conflict with the theming branch, wondering if you should use/target that branch instead. Here's what it looks like in the new branch (to be merged soon) ->
https://github.com/apache/superset/blob/template_less/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx#L53-L57
Note sure why we're missing the dark tones in that branch, maybe a bad merge conflict resolve from a previous PR? Seems like a regression... Glad to catch it before it hits.
Anyhow we should think about what it'll look like in that branch, should probably find matching color using the antd-v5 tokens. Here's the before/after on the tokens now that we use "semantic shades":
<img width="1379" alt="Screenshot 2025-06-04 at 4 27 08 PM" src="https://github.com/user-attachments/assets/01165861-c7a8-40ae-ab58-7c5191922e38" />
--
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
Re: [PR] feat(charts): Adds "info" and "info dark" (blue colors) values to edit formatter color scheme [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #33601:
URL: https://github.com/apache/superset/pull/33601#issuecomment-3180935713
Converting to DRAFT until merge conflict is resolved. Would approve otherwise. Feel free to rebase and mark as "ready for review"
--
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
Re: [PR] feat(charts): Adds "info" and "info dark" (blue colors) values to edit formatter color scheme [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #33601:
URL: https://github.com/apache/superset/pull/33601#issuecomment-2950163959
Some input/examples from GTP on using lighten and darken out of `tinycolor2` : https://chatgpt.com/share/684335cb-41c4-8010-b2ca-e1456283c376
I think ideally @EmmanuelCbd if you can spare the time, the ideal way to contribute the related features here would be to work off of the `template_less` branch (and target it with a new PR).
The new PR would:
- use tinycolor to create variations off of the system colors (`colorInfo`, `colorWarning`, ...) and generally match the brightness currently used
- ensure backwards compatibility with the options now served in `master`
- add the new color(s) (`colorInfo`) and maybe others off of the new theming system
--
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
Re: [PR] feat(charts): Adds "info" and "info dark" (blue colors) values to edit formatter color scheme [superset]
Posted by "EmmanuelCbd (via GitHub)" <gi...@apache.org>.
EmmanuelCbd commented on PR #33601:
URL: https://github.com/apache/superset/pull/33601#issuecomment-3883240329
> Closing due to contributor inactivity for several months. Feel free to reopen if/when warranted, and thank you for the contribution!
Yes, sorry for that. I'm currently overloaded at work and didn't had much time to give a look at mistercrunch directions.
Will check if still needed on newer version.
Regards
--
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
Re: [PR] feat(charts): Adds "info" and "info dark" (blue colors) values to edit formatter color scheme [superset]
Posted by "EmmanuelCbd (via GitHub)" <gi...@apache.org>.
EmmanuelCbd commented on PR #33601:
URL: https://github.com/apache/superset/pull/33601#issuecomment-2929130989
Hi @michael-s-molina , @rusackas ,
Thanks for reviewing.
I used default theme color:
theme.colors.info.light1
theme.colors.info.dark1
Just added those into the list, following the other ones, I didn't think it would have impact anywhere.
--
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
Re: [PR] feat(charts): Adds "info" and "info dark" (blue colors) values to edit formatter color scheme [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #33601:
URL: https://github.com/apache/superset/pull/33601#issuecomment-2941960910
Two options: abuse the semantic meaning of the tokens and use things like `colorErrorBorderHover` (for light) and `colorErrorText` for dark. Or we use a RGB library/function and constant (say +10%, -10%) to lighten/darken the base color (`colorError`). The second option seems more right and you can use `tinycolor2`.
Knowing there's a regression in the branch and that it's a bit much for the person handling the eventual merge conflict, I'd recommend taking on the change on that branch and getting things right there using `tinycolor2` to apply the right transformation. Ideally we apply the same/right amount to darken/lighten in a way that roughly matches the color we happen to have now.
@EmmanuelCbd are you willing to take this on?
--
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