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
style: replace broken glyphs with font-awesome #10123
Conversation
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
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.
one question, otherwise lgtm.
I think we'll need to go through the chart plugins and fix these too, since i know tables have been broken because of this.
cc @michellethomas who was tracking this bug
@@ -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 }); |
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.
Is this related to the glyph changes? is it intentional?
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.
That was a minor bug I uncovered, as I change the icons to caret-(left|right)
There's a new table chart coming soon. Hopefully we don't have to have another fix once the drop-in replacement is merged. P.S. I was using react-icons for the new table chart. With tree shaking properly configured, these svg icons packs should work very well. |
@ktmud @etr2460 it'd be nice to be able to |
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 |
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
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
before
after