-
Notifications
You must be signed in to change notification settings - Fork 13.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
[sql_lab]Disabled run query button if sql query editor is empty #4728
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4728 +/- ##
==========================================
+ Coverage 72.22% 72.38% +0.15%
==========================================
Files 204 205 +1
Lines 15323 15374 +51
Branches 1180 1182 +2
==========================================
+ Hits 11067 11128 +61
+ Misses 4253 4243 -10
Partials 3 3
Continue to review full report at Codecov.
|
@@ -43,6 +44,7 @@ export default function RunQueryActionButton(props) { | |||
onClick={() => props.runQuery(true)} | |||
key="run-async-btn" | |||
tooltip={t('Run query asynchronously')} | |||
disabled={!props.sql.trim()} |
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 confused as to why the linter isn't asking for sql
to be in propTypes
. I wanted to make sure it's entered as a string
and can never be null
through a defaultProps = ''
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 have made changes, please review 👍
I didn't understand your comment, can you please elaborate.
…On Tue 3 Apr, 2018, 10:13 AM Maxime Beauchemin, ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
In superset/assets/javascripts/SqlLab/components/RunQueryActionButton.jsx
<#4728 (comment)>
:
> @@ -43,6 +44,7 @@ export default function RunQueryActionButton(props) {
onClick={() => props.runQuery(true)}
key="run-async-btn"
tooltip={t('Run query asynchronously')}
+ disabled={!props.sql.trim()}
I'm confused as to why the linter isn't asking for sql to be in propTypes.
I wanted to make sure it's entered as a string and can never be null
through a defaultProps = ''
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#4728 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AWLeFKQiCQRRzNVaFtppGyPqfk1C2fpmks5tkv31gaJpZM4TCy38>
.
|
I got it, I will add propsType for sql as well, Thanks |
…he#4728) * Disabled run query button if sql query editor is empty * Removing unnecessary white space * Fix failing test for sql props * Adding sql variable into propTypes and defaultProps
…he#4728) * Disabled run query button if sql query editor is empty * Removing unnecessary white space * Fix failing test for sql props * Adding sql variable into propTypes and defaultProps
…he#4728) * Disabled run query button if sql query editor is empty * Removing unnecessary white space * Fix failing test for sql props * Adding sql variable into propTypes and defaultProps
Fix
#2102
Disabled run button if sql query is empty.