-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
[SIP-5] Revise markup.js and iframe.js #5672
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #5672 +/- ##
==========================================
- Coverage 63.49% 63.47% -0.03%
==========================================
Files 360 360
Lines 22889 22894 +5
Branches 2549 2550 +1
==========================================
- Hits 14534 14532 -2
- Misses 8340 8347 +7
Partials 15 15
Continue to review full report at Codecov.
|
LGTM |
hopefully we'll be deprecating these soon but also lgtm 👍 |
kristw
added a commit
to kristw/incubator-superset
that referenced
this pull request
Aug 27, 2018
* Do not call slice.xxx * remove iframe id * remove old code * use import instead of require * update iframe.js (cherry picked from commit 9fb28b5)
kristw
added a commit
to kristw/incubator-superset
that referenced
this pull request
Aug 27, 2018
* Do not call slice.xxx * remove iframe id * remove old code * use import instead of require * update iframe.js (cherry picked from commit 9fb28b5)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Oct 12, 2018
* Do not call slice.xxx * remove iframe id * remove old code * use import instead of require * update iframe.js
wenchma
pushed a commit
to wenchma/incubator-superset
that referenced
this pull request
Nov 16, 2018
* Do not call slice.xxx * remove iframe id * remove old code * use import instead of require * update iframe.js
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
slice.xxx
iframe
element in js and keep it for later use instead of settinginnerHTML
to create iframe and later lookup iframe by id. Remove iframe id.$('#code').attr('rows', '15')
because the element with idcode
does not exist so this block does nothing. Also make this component independent from jqueryimport
/export
instead ofrequire
andmodule.exports
overflow:auto
. The previous code has a bug and did not set the style successfully.render_template
andMustache
dependency fromChart.jsx
Test
Ran a development instance with the code above and verified with production instance that they produce the same results.