Skip to content

[sqllab] run only the part of the query that is selected#1479

Merged
mistercrunch merged 1 commit intoapache:masterfrom
mistercrunch:run_selection
Nov 1, 2016
Merged

[sqllab] run only the part of the query that is selected#1479
mistercrunch merged 1 commit intoapache:masterfrom
mistercrunch:run_selection

Conversation

@mistercrunch
Copy link
Member

@mistercrunch mistercrunch commented Oct 29, 2016

screen shot 2016-10-28 at 6 25 54 pm

screen shot 2016-10-28 at 6 25 46 pm

Fixes https://github.com//issues/1427

@bkyryliuk
Copy link
Member

Lovely, looks good to me!
🚀

Do you plan to support run async the selected text?

@mistercrunch
Copy link
Member Author

Async also works with selected text, just the same as Run Query

@mistercrunch mistercrunch merged commit 4023f32 into apache:master Nov 1, 2016
@mistercrunch mistercrunch deleted the run_selection branch November 1, 2016 04:07
@ascott
Copy link

ascott commented Nov 2, 2016

i'm curious about the color change for the buttons. i think updating only the text in the button should be enough of a ui update so users know they can run just the selected query. @mistercrunch what was your thinking with changing the color as well?


render() {
let runButtons = [];
let runText = 'Run Query';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be great to move the run buttons out of this component and into it's own component. there's a lot happening in this component and with change, more happening with the run buttons.

@mistercrunch
Copy link
Member Author

The warning color was to draw attention that 1-the feature exists, and 2-that it's essentially a different button at this point, with a different behavior. I though of having a brand new extra button "Run Selection" that would be disabled when not selected, but with potentially "Run Query" and "Run Async" already, that multiplies the buttons to 4 buttons (+"Run Selection Async").

I think changing the button text only is too subtle. We need something very clear and perhaps more discoverable, though as it is currently (with the color) I feel like a user would discover the feature quite quickly, most likely in its first heavy session of usage.

I've seen UI features that will have a temporary "flash" to draw attention that something has changed, away from the action trigger, and go back to its final state quickly. An example of that type of UX would be if you add a TableElement, we could make its background yellow, fading back to default color over 3-5 seconds.

@ascott
Copy link

ascott commented Nov 2, 2016

i can see your point about making it more obvious, but i think if someone is selecting part of the query with the intent to run just the selected part, and then looks at the run query button and and it now says run selected query, that would be clear enough. seems like this functionality is also fairly common so a user probably has an idea about it before the select part of the query.

@mistercrunch
Copy link
Member Author

The color change augments the chance of catching the user's eye for this feature's discovery. The label change by itself on a button is pretty subtle, it'd be easy not to notice it. Are you concerned that the color change makes it look to "alarming"?

@ascott
Copy link

ascott commented Nov 7, 2016

yeah, it seems to send a difference message with the warning color.. i also think it's usage is just not consistent with what is expected. in any case, i don't think we should spend more cycles on this for now, if you feel strongly about making the color change, let's leave as is. 👌

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.13.0 First shipped in 0.13.0 labels Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.13.0 First shipped in 0.13.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants