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/06/21 06:02:41 UTC
[GitHub] [incubator-superset] mistercrunch opened a new pull request #10123: style: replace broken glyphs with font-awesome
mistercrunch opened a new pull request #10123:
URL: https://github.com/apache/incubator-superset/pull/10123
Glyphicons stopped working recently, not sure why, but let's get rid of
them and double down on font-awesome that we use a lot more in the
codebase. There's only a few instances of glyphicons and they all are
broken ATM.
Also a few other minor style tweaks
<img width="316" alt="Screen Shot 2020-06-20 at 10 58 58 PM" src="https://user-images.githubusercontent.com/487433/85217912-19cc6980-b34a-11ea-8eed-1ad7318cd51e.png">
<img width="386" alt="Screen Shot 2020-06-20 at 10 44 58 PM" src="https://user-images.githubusercontent.com/487433/85217913-1a650000-b34a-11ea-8ee8-776598beb74f.png">
<img width="419" alt="Screen Shot 2020-06-20 at 10 42 56 PM" src="https://user-images.githubusercontent.com/487433/85217915-1b962d00-b34a-11ea-8f0d-7068f527683e.png">
<img width="193" alt="Screen Shot 2020-06-20 at 10 42 35 PM" src="https://user-images.githubusercontent.com/487433/85217916-1c2ec380-b34a-11ea-8fa8-8c0f7337ce45.png">
----------------------------------------------------------------
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] etr2460 commented on pull request #10123: style: replace broken glyphs with font-awesome
Posted by GitBox <gi...@apache.org>.
etr2460 commented on pull request #10123:
URL: https://github.com/apache/incubator-superset/pull/10123#issuecomment-647273503
I agree @mistercrunch, not having i18n work for superset-ui is a real pain :/ That said, getting all the strings working/fixed in the main repo is enough of a task, not even considering the side repos. There are a lot of mistaken usages of `t` strings that i've been meaning to clean up
----------------------------------------------------------------
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] mistercrunch commented on pull request #10123: style: replace broken glyphs with font-awesome
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10123:
URL: https://github.com/apache/incubator-superset/pull/10123#issuecomment-647270088
@ktmud `SIP-34` provided icons as SVG, they're starting to land. Consistency would be great eventually. Between glyphicons, SIP-34 svg, react-icons, font-awesome, ... Maybe we need a `superset-ui-icons` package :)
@etr2460 it'd be nice to be able to `grep` across all superset code. I `git grep` a lot and totally miss out on other repos. Also noticed that the documented `i18n` command currently doesn't inventorize the strings in `superset-ui*`, I spent a bit of time trying to fix this messing with `babel.cfg` (the python i18n babe-thing, not the JS one), with symlink, but nothing worked. Now I'm thinking git submodules OR a big mono-repo may work.
----------------------------------------------------------------
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] etr2460 commented on a change in pull request #10123: style: replace broken glyphs with font-awesome
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #10123:
URL: https://github.com/apache/incubator-superset/pull/10123#discussion_r443245260
##########
File path: superset-frontend/src/explore/components/AdhocMetricOption.jsx
##########
@@ -53,7 +53,7 @@ export default class AdhocMetricOption extends React.PureComponent {
// once the overlay has been opened, the metric/filter will never be
// considered new again.
this.props.adhocMetric.isNew = false;
- this.setState({ overlayShown: false });
+ this.setState({ overlayShown: true });
Review comment:
Is this related to the glyph changes? is it intentional?
----------------------------------------------------------------
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] mistercrunch commented on a change in pull request #10123: style: replace broken glyphs with font-awesome
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #10123:
URL: https://github.com/apache/incubator-superset/pull/10123#discussion_r443311546
##########
File path: superset-frontend/src/explore/components/AdhocMetricOption.jsx
##########
@@ -53,7 +53,7 @@ export default class AdhocMetricOption extends React.PureComponent {
// once the overlay has been opened, the metric/filter will never be
// considered new again.
this.props.adhocMetric.isNew = false;
- this.setState({ overlayShown: false });
+ this.setState({ overlayShown: true });
Review comment:
That was a minor bug I uncovered, as I change the icons to `caret-(left|right)`
----------------------------------------------------------------
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] mistercrunch merged pull request #10123: style: replace broken glyphs with font-awesome
Posted by GitBox <gi...@apache.org>.
mistercrunch merged pull request #10123:
URL: https://github.com/apache/incubator-superset/pull/10123
----------------------------------------------------------------
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] ktmud commented on pull request #10123: style: replace broken glyphs with font-awesome
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10123:
URL: https://github.com/apache/incubator-superset/pull/10123#issuecomment-647201686
There's a new table chart coming soon. Hopefully we don't have to have another fix once [the drop-in replacement](https://github.com/apache-superset/superset-ui/pull/623) is merged.
P.S. I was using [react-icons](https://react-icons.github.io/react-icons/) for the new table chart. With tree shaking properly configured, these svg icons packs should work very well.
----------------------------------------------------------------
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] mistercrunch edited a comment on pull request #10123: style: replace broken glyphs with font-awesome
Posted by GitBox <gi...@apache.org>.
mistercrunch edited a comment on pull request #10123:
URL: https://github.com/apache/incubator-superset/pull/10123#issuecomment-647270088
@ktmud `SIP-34` provided icons as SVG, they're starting to land. Consistency would be great eventually. Between glyphicons, SIP-34 svg, react-icons, font-awesome, ... Maybe we need a `superset-ui-icons` package that brings it back together and offer a common interface to all this that we can rationalize over time.
@etr2460 it'd be nice to be able to `grep` across all superset code. I `git grep` a lot and totally miss out on other repos. Also noticed that the documented `i18n` command currently doesn't inventorize the strings in `superset-ui*`, I spent a bit of time trying to fix this messing with `babel.cfg` (the python i18n babe-thing, not the JS one), with symlink, but nothing worked. Now I'm thinking git submodules OR a big mono-repo may work.
----------------------------------------------------------------
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