You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/10/02 04:18:49 UTC
[GitHub] [incubator-superset] nytai opened a new pull request #11139: style: override ace edit error icon
nytai opened a new pull request #11139:
URL: https://github.com/apache/incubator-superset/pull/11139
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
before:
<img width="894" alt="Screen Shot 2020-10-01 at 9 15 10 PM" src="https://user-images.githubusercontent.com/10255196/94887568-88ca5b80-042b-11eb-9874-4d186a2d27df.png">
after:
<img width="388" alt="Screen Shot 2020-10-01 at 9 15 49 PM" src="https://user-images.githubusercontent.com/10255196/94887589-967fe100-042b-11eb-85e0-6cbdb2aa8b9a.png">
### TEST PLAN
<!--- What steps should be taken to 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:
- [x] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] 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.
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
[GitHub] [incubator-superset] nytai commented on pull request #11139: style: override ace edit error icon
Posted by GitBox <gi...@apache.org>.
nytai commented on pull request #11139:
URL: https://github.com/apache/incubator-superset/pull/11139#issuecomment-702839097
I spent some time trying to get the mask to work, but instead the line number disappeared and the entire svg assets was rendered as red.
<img width="327" alt="Screen Shot 2020-10-02 at 9 39 23 AM" src="https://user-images.githubusercontent.com/10255196/94947940-53f1ef00-0493-11eb-93b1-e35b5ef9bdeb.png">
This change is really only meant to be a quick fix for overriding a 3rd party library. I think this case is the exception rather than the norm. I'd rather not spend too much time optimizing for a problem we don't yet have. We can always revisit this if it does become an issue.
----------------------------------------------------------------
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.
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
[GitHub] [incubator-superset] nytai merged pull request #11139: style: override ace edit error icon
Posted by GitBox <gi...@apache.org>.
nytai merged pull request #11139:
URL: https://github.com/apache/incubator-superset/pull/11139
----------------------------------------------------------------
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.
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
[GitHub] [incubator-superset] rusackas commented on pull request #11139: style: override ace edit error icon
Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #11139:
URL: https://github.com/apache/incubator-superset/pull/11139#issuecomment-702533213
'error-solid-small' and 'error-solid' are already available in the Icon component, and respect `currentColor` - would that be sufficient for this use case?
----------------------------------------------------------------
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.
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
[GitHub] [incubator-superset] nytai commented on pull request #11139: style: override ace edit error icon
Posted by GitBox <gi...@apache.org>.
nytai commented on pull request #11139:
URL: https://github.com/apache/incubator-superset/pull/11139#issuecomment-702536333
@rusackas I don't it's possible to override a background-image css property any other way. Fill color cannot be applied to an svg when it's a background image (it's not in the dom).
----------------------------------------------------------------
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.
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
[GitHub] [incubator-superset] nytai edited a comment on pull request #11139: style: override ace edit error icon
Posted by GitBox <gi...@apache.org>.
nytai edited a comment on pull request #11139:
URL: https://github.com/apache/incubator-superset/pull/11139#issuecomment-702536333
@rusackas I don't think it's possible to override a background-image css property any other way. Fill color cannot be applied to an svg when it's a background image (not in the dom).
----------------------------------------------------------------
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.
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
[GitHub] [incubator-superset] rusackas commented on pull request #11139: style: override ace edit error icon
Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #11139:
URL: https://github.com/apache/incubator-superset/pull/11139#issuecomment-702541127
@nytai correct... but I'd be half tempted to try the CSS mask trick outlined here:
https://codepen.io/noahblon/post/coloring-svgs-in-css-background-images
It won't work in IE, so those users simply won't see red. But if this _does_ work well enough, then it can even be done in Emotion to use the Theme color. Fancy!
Probably getting a bit into the weeds, but if that kind of hack doesn't work, we should at least find a way to namespace these around existing icons (from the Design System). Maybe then we could even come up with a cool script to generate all the semantic color flavors of each Design System icon.
----------------------------------------------------------------
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.
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