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

Add GUI specific classes for browser interaction #8352

Merged
merged 16 commits into from Nov 4, 2018

Conversation

nyalldawson
Copy link
Collaborator

This PR introduces a new method for populating the browser dock widget context menu, by adding a registry of QgsDataItemGuiProviders which can be used to populate and modify the right click context menu for items.

Currently, browser context menu population is an ugly mix of gui specific code sitting incorrectly within the core library (with the limitations associated with that, e.g. no access to QGIS gui widgets, message bar, etc), or specifically hard coded into the browser dock widget (not extensible, accessible by plugins, etc. Also no access to application library or provider libraries here because it's all done from the gui library).

This also addresses some shortcomings in the current API, by:

  1. passing a context to the providers during menu creation, allowing actions to access properties like the message bar

  2. passing a full list of selected items, allowing actions which operate on the complete selection (e.g. delete selected layers from geopackage)

@nyalldawson nyalldawson added the API API improvement only, no visible user interface changes label Oct 27, 2018
@nyalldawson nyalldawson added this to the 3.6 milestone Oct 27, 2018
@nyalldawson
Copy link
Collaborator Author

@elpaso @wonder-sk Very keen for your feedback here!

@elpaso
Copy link
Contributor

elpaso commented Oct 27, 2018

Wonderful! I'll check it out ASAP.

@nyalldawson
Copy link
Collaborator Author

nyalldawson commented Oct 27, 2018

Here's a rough Todo list:

  • move all actions out of core and to providers
  • move hard coded actions out of the dock widget code
  • move geonode specific actions to their own provider
  • add handledrop method to gui providers
  • move browser item widget handling to gui providers
  • update provider data items to use message bar for feedback instead of messageboxes

@NathanW2
Copy link
Member

NathanW2 commented Oct 27, 2018 via email

Copy link
Contributor

@elpaso elpaso left a comment

Choose a reason for hiding this comment

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

Great! I think this covers the use cases for selected items and gives us a lot of flexibility we need for building context menus.

@nyalldawson
Copy link
Collaborator Author

@wonder-sk can I get your opinion here too please?

@wonder-sk
Copy link
Member

Looking good - nice cleanup!

Few thoughts:

  • shall we deprecate actions() and handleDoubleClick()?
  • as far as I understand, all the things supported by providers are still related to the browser dock widget. What do you think about having a possibility in browser tree view API to pick which providers should be used?
  • the note "providers could potentially alter menus and actions added by other providers" is a bit worrying - is that mostly theoretical idea or are some plans for that? I guess this would require a particular order of providers to work

@nyalldawson
Copy link
Collaborator Author

@wonder-sk

shall we deprecate actions() and handleDoubleClick()?

I've thought about this, but I think they should be left to make the api easier for PyQGIS based items, where the mix of core/gui concepts is less harmful.

as far as I understand, all the things supported by providers are still related to the browser dock widget. What do you think about having a possibility in browser tree view API to pick which providers should be used?

I think we should. At the moment QgsBrowserTreeView is a very basic class, only adding the persistence of expanded/collapsed state to QTreeView, and there's definetly potential to start moving the valuable stuff from the dock to the view. This would also benefit @elpaso's recent browser based dialog. I'll defer this to follow up PRs.

the note "providers could potentially alter menus and actions added by other providers" is a bit worrying - is that mostly theoretical idea or are some plans for that?

Totally theoretical. They "could"... whether they "should" is another question. But it is kind of nice for plugins have the potential to hook in here and mangle these menus however they see fit. With great power comes yadayadayada.

I guess this would require a particular order of providers to work

Yep. I considered adding a "priority" method to the API, but I'll leave that until we really need it. I can potentially see us wanting this when we want to ensure a logical ordering of menu items, and e.g. require some provider to be able to insert its items in between the items added by another provider. For now, it's probably enough to just do what's done here and process them in the order added.

These providers will be used to control how the browser data items
behave within GUI, and to allow separation of GUI related
properties of browser items from the core code.

A new registry QgsDataItemGuiProviderRegistry has been created
(modeled off QgsDataItemProviderRegistry), with an application
wide instance available from QgsGui::instance()->dataItemGuiProviderRegistry()
so that actions can operate on ALL selected items

E.g. delete multiple layers from a geopackage at once
...by implementing dataItemGuiProviders()
And remove more QWidget imports from core library
With option to create a new geopackage or shapefile in the
clicked directory
…ider

And move handling of layer/project file double clicks in browser dock
from gui->app
in browser

With options to
- load project
- open project file properties (operating system file properties
dialog that is, on supported platforms only)
No longer required, and of limited value anyway
@nyalldawson nyalldawson closed this Nov 4, 2018
@nyalldawson nyalldawson reopened this Nov 4, 2018
@wonder-sk
Copy link
Member

This all sounds good :-)

@nyalldawson nyalldawson merged commit 8c5f795 into qgis:master Nov 4, 2018
@nyalldawson nyalldawson deleted the data_item_gui branch November 4, 2018 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API API improvement only, no visible user interface changes Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants