-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
fix: broken glyphicons used in react-json-schema #10267
Conversation
a4cd08e
to
42bb6b7
Compare
Codecov Report
@@ Coverage Diff @@
## master #10267 +/- ##
==========================================
- Coverage 70.18% 65.28% -4.90%
==========================================
Files 598 598
Lines 32004 31994 -10
Branches 3236 3236
==========================================
- Hits 22462 20888 -1574
- Misses 9437 10925 +1488
- Partials 105 181 +76
Continue to review full report at Codecov.
|
On a separate note, we can probably start thinking about replacing all code editor instances with monaco-editor, the editor behind VS Code, which also supports json schemas. It's much more actively maintained than Ace/Brace and has many more advanced features. |
I'm working on a talk for the Airflow Summit "Advanced Apache Superset for Data Engineers" and showing the "Schedule Query" feature that Beto contributed a while back (behind a feature flag). I found that the glyphicons used in `react-json-schema` are broken and came up with an easy fix. Also other minor tweaks on the feature.
42bb6b7
to
67fa33a
Compare
@@ -162,6 +162,7 @@ class SavedQueryViewApi(SavedQueryView): # pylint: disable=too-many-ancestors | |||
"description", | |||
"sql", | |||
"extra_json", | |||
"extra", |
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.
I'm surprised to see both extra
and extra_json
- what's the difference/motivation?
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.
extra_json
is a string containing the serialized JSON, extra
is the actual nested data structure. While I know the private API does not use extra_json
, I assume that people in the wild (notably at Lyft where @betodealmeida shipped the feature) are very likely to use it.
Clearly it's not great to have JSON inception (serialized JSON inside of JSON)
I could have deprecated extra_json
in favor of extra
, but would need to do change management around it - likely just adding a note in UPDATING.md
. Eventually we should move this under /api/v1
but, maybe a good time to do this.
I'm working on a talk for the Airflow Summit "Advanced Apache Superset for Data Engineers" and showing the "Schedule Query" feature that Beto contributed a while back (behind a feature flag). I found that the glyphicons used in `react-json-schema` are broken and came up with an easy fix. Also other minor tweaks on the feature.
I'm working on a talk for the Airflow Summit
"Advanced Apache Superset for Data Engineers" and showing the "Schedule
Query" feature that Beto contributed a while back (behind a feature flag).
I found that the glyphicons used in
react-json-schema
are broken andcame up with an easy fix.
Also other minor tweaks on the feature.
before
after