[Browser] Reorder the buttons position in the Browser SQL editor and only execute the selected SQL text

[agiudiceandrea]

Description

This PR:

• reorders the buttons position in the Browser SQL editor dialog window
• allows to only execute the selected SQL text as using the DB Manger

---

Old description
This PR:

• swaps the "Execute" and "Clear" buttons position in the Browser SQL editor dialog window in order to make it more similar to the DB Manger SQL editor dialog window
  before
  
  after
  
  @gioman, is it OK for you?

• allows to only execute the selected SQL text as using the DB Manger

Not sure if this should be considered a bug fix (and thus be backported) or a feature.

Fixes #57202.

PS: @DelazJ, it looks like a description of the Browser SQL editor dialog window is missing in the docs. Doesn't it?

[agiudiceandrea]

I've used selectedText().length() > 0 instead of hasSelectedText() after looking at c3d9f10 and https://issues.qgis.org/issues/12193. Maybe it is no longer needed now or not needed here...

[nyalldawson]

I can't find the discussion now, but I vaguely recall some past conversation where the "run only selected text" change was controversial. @elpaso does this ring any bells?

[elpaso]

@agiudiceandrea for what it worth I think it is more intuitive having the execution related buttons grouped together (execute and stop) while the clear button is editing related, having the stop button right aside the execute button may also help in case a user wanted to quickly halt the execution right after pushing execute. But I have no strong opinions (in other words: I don't care).

@nyalldawson it does but I can't find the old discussion now.
About the "execute selected" IIRC the proposal was to make it explicit by adding an "Execute selected" button which is disabled if there is no selection. It would be even better if the SQL parser could detect a valid query before enabling the button (see: QgsSQLStatement).

[gioman]

@agiudiceandrea much better thanks! I don't have a string opinion in general too, but to stay consistent with DB manager is very important: I lost a complex query that took hours to refine because I hit "clear" instead of "execute" just because I'm used (we are) to how buttons are ordered in DB manager, and to find also that "undo" is not supported (I apologized for my harsh comment here #57202 (comment) but as you can imagine that was very very annoying to say the least).

[elpaso]

@agiudiceandrea much better thanks! I don't have a string opinion in general too, but to stay consistent with DB manager is very important: I lost a complex query that took hours to refine because I hit "clear" instead of "execute" just because I'm used (we are) to how buttons are ordered in DB manager, and to find also that "undo" is not supported (I apologized for my harsh comment here #57202 (comment) but as you can imagine that was very very annoying to say the least).

While I can sympathize with you for your difficulties to adapt to a different interface, I don't think that copying the DB manager interface is a good enough strategy.
The question here is what makes more sense from a UX point of view (which may well be the DB-manager solution).

About the attitude, I strongly advice you to soften your tones: it is usually much more effective to use a gentle and polite approach when asking people to volunteer their time to do something for you.

[gioman]

I can't find the discussion now, but I vaguely recall some past conversation where the "run only selected text" change was controversial. @elpaso does this ring any bells?

I found strange to be controversial: I think is pretty common for final users (at least for me) to use the SQL console (any SQL console) as a place where to build by steps queries (plural) and not only where to run a final one written elsewhere. This makes really necessary the possibility to run just selected rows (as seen also in any other SQL console I have used like pgadmin4). A "execute only selected" would clearly address this but also maybe not necessary if we assume that running only selected rows is what anyone expects anyway.

[gioman]

About the attitude, I strongly advice you to soften your tones: it is usually much more effective to use a gentle and polite approach when asking people to volunteer their time to do something for you.

@elpaso I apologized before this reply from you, This was unnecessary as it was my comment. I know well how things work and you know I know. I was just irritated and I know you understand what I mean.

[DelazJ]

PS: @DelazJ, it looks like a description of the Browser SQL editor dialog window is missing in the docs. Doesn't it?

Indeed!

swaps the "Execute" and "Clear" buttons position in the Browser SQL editor dialog window in order to make it more similar to the DB Manger SQL editor dialog window

from developer guides

Keep harmful actions away from harmless ones: If you have actions for ‘delete’, ‘remove’ etc, try to impose adequate space between the harmful action and innocuous actions so that the users is less likely to inadvertently click on the harmful action.

and in this case I would consider "clear" button as a harmful button, compared to the others.

[gioman]

While I can sympathize with you for your difficulties to adapt to a different interface,

As a long time and heavy db manager user I have the execute button on the left imprinted in my brain, and probably I'm not alone.

I don't think that copying the DB manager interface is a good enough strategy.
The question here is what makes more sense from a UX point of view (which may well be the DB-manager solution).

Fair enough.

[agiudiceandrea]

Thanks for all your comments and suggestions!
I've found the previous discussion at #55200.

I agree with @elpaso #57211 (comment) about the position of the "Stop" button and with @nyalldawson #55200 (comment) about the "Execute" / "Execute Selection" button.

Please see a PoC with the new behaviour and buttons design:

I also agree with @Gustry suggestions in comment #55200 (comment).

[elpaso]

Please see a PoC with the new behaviour and buttons design:

Looks good to me.

[Gustry]

Thanks for check @agiudiceandrea

This POC looks good as well. It makes it clear.

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[nyalldawson]

@agiudiceandrea the proof of concept looks good to me! Will you update this pr to match?

[github-actions]

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 89e89de)

[agiudiceandrea]

@agiudiceandrea the proof of concept looks good to me! Will you update this pr to match?

I apologize for the delay. I've force-pushed the updates as in the PoC and I've also updated the PR title and description accordingly.

I've not implemented the suggestions made by @Gustry in comment #55200 (comment), since I've not found a not ugly way to make such behaviour clear in the GUI and I'm also not completely convinced it should be the correct behaviour (nor I'm convinced the current behaviour is totally correct).