Skip to content
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

floating widget for advanced input next to the cursor #9988

Merged
merged 7 commits into from
May 19, 2019

Conversation

olivierdalang
Copy link
Contributor

@olivierdalang olivierdalang commented May 13, 2019

Description

This PR adds a toggleable widget that shows the advanced input coordinates next to the cursor, which is more comfortable than the dock widget which may be far away (esp. on big screens).

On small screens, it is also nice since it is now possible to use advanced input even with the Advanced Digitizing dock widget hidden.

Sponsored by Kanton Schaffhausen in collaboration with OPENGIS.ch

floater

Checklist

  • Commit messages are descriptive and explain the rationale for changes
  • (N/A) Commits which fix bugs include fixes #11111 in the commit message next to the description
  • Commits which add new features are tagged with [FEATURE] in the commit message
  • Commits which change the UI or existing user workflows are tagged with [needs-docs] in the commit message and contain sufficient information in the commit message to be documented
  • I have read the QGIS Coding Standards and this PR complies with them
  • This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
  • (N/A) New unit tests have been added for core changes
  • I have run the scripts/prepare-commit.sh script before each commit

…next to the mouse

Sponsored by Kanton Schaffhausen in collaboration with OPENGIS.ch
@3nids
Copy link
Member

3nids commented May 13, 2019

nice addition!
since it's a setting, what about adding that to the existing menu?

@olivierdalang olivierdalang mentioned this pull request May 13, 2019
8 tasks
@m-kuhn m-kuhn added the Feature label May 13, 2019
@m-kuhn m-kuhn changed the title [FEATURE] floating widget for advanced input next to the cursor floating widget for advanced input next to the cursor May 13, 2019
@nyalldawson
Copy link
Collaborator

This is very nice!

I gave the build a test run and one thing which should be done is to ensure that the constraint value is evaluated and set whenever it loses focus. E.g, in the main digitizing panel pressing "d" and entering "1000/7", press tab, results in 1000/7 being immediately evaluated. In floating mode pressing "d", 1000/7, "a" leaves the "1000/7" text in the distance row instead of evaluating the expression to a value.

Another issue is that the text becomes unreadable on a dark map background -- either the background rect needs to cover the whole text (including the row headers), or the background could be removed in favour of drawing the text with a white buffer (preferable in my opinion).

Lastly -- shouldn't we just make this default behavior? When would you ever not want it?

@olivierdalang
Copy link
Contributor Author

Thanks for the inputs !

@3nids
since it's a setting, what about adding that to the existing menu?

You mean additionally to the current button, or instead of it ? I think it is nice to keep the toggle button relatively accessible (see below answer to Nyall's last comment), and if we keep it, the setting button may be a bit useless...

@nyalldawson
I gave the build a test run and one thing which should be done is to ensure that the constraint value is evaluated and set whenever it loses focus. E.g, in the main digitising panel pressing "d" and entering "1000/7", press tab, results in 1000/7 being immediately evaluated. In floating mode pressing "d", 1000/7, "a" leaves the "1000/7" text in the distance row instead of evaluating the expression to a value.

Ok, I see what you mean. I'll try to fix this. Currently, the behaviour you describe only happens when the field is locked. Arguably, we should go even further, and lock the fields automatically when edited (as entering a value without locking has no effect). This would also solve a similar issue (that also happen in the DockWidget), where hitting alt+a to lock the angle after having changed the value actually locks the current angle instead of the value in the edit widget.

@nyalldawson
Another issue is that the text becomes unreadable on a dark map background -- either the background rect needs to cover the whole text (including the row headers), or the background could be removed in favour of drawing the text with a white buffer (preferable in my opinion).

Yes true, I mostly tested on bright backgrounds. Would you have a pointer to an elegant/easy way to draw a white buffer ? I fear it's not that simple out of the box, as it seems Qt's CSS doesn't allow it (meaning it requires overriding the paint functions). Would a semi-transparent background do ?

@nyalldawson
Lastly -- shouldn't we just make this default behavior? When would you ever not want it?

Not sure, there are moments when you really don't mind about coordinates (e.g. manually digitising from a raster), and where it is just a distraction.

@olivierdalang
Copy link
Contributor Author

Ok just pushed some changes.

The behaviour should now exactly match the behaviour of the DockWidget. The implementation is a bit hackish, as initial implementation of the DockWidget wasn't really thought for this kind of hooks, and I didn't want to make too much changes. Ideally (maybe for 3.10 and depending on resources) this could be refactored, so the we separate completely the logic and UI.

I also updated the style of the floater so that it looks better and also works on dark background. I didn't try to do buffers so far to keep it simple. Here's how it looks :

a b c

Let me know what you think,

Olivier

@NathanW2
Copy link
Member

NathanW2 commented May 15, 2019 via email

@m-kuhn
Copy link
Member

m-kuhn commented May 15, 2019

Ideally (maybe for 3.10 and depending on resources) this could be refactored, so the we separate completely the logic and UI.

In this case, could you add a note to the docstring of the new methods to tag them as instable so we can remove them again without troubles.

After that it's good to merge for me

@olivierdalang
Copy link
Contributor Author

@m-kuhn Done (added \note unstable API (will likely change) on all public methods)

@m-kuhn m-kuhn merged commit 03faaa3 into qgis:master May 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants