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 layer button #4629

Merged
merged 34 commits into from Jun 2, 2017

Conversation

Projects
None yet
@elpaso
Copy link
Contributor

commented May 24, 2017

Description

The unified add layers button and dialog as discussed in Essen (see: https://docs.google.com/document/d/1aMX9jOfl10q8oETRzOHSHnRpHCc7UJ1dDnCCq3VyTAY/edit )
unibutton-3

TODO

  • Raster layers
  • Virtual layers
  • Delimited text layers
  • GUI Styles
  • Dialogs sizing
  • Left icon bar collapsible
  • Delimited text dialog refactoring (this is really too wide and a bit confusing)
  • Vector layers dialog sizing (fixed width: does not adapt to widget container)
  • Persist dialog status (active tab etc.)
  • Settings option for modeless Dialog
  • Methods and key bindings to open the dialog on a particular selection page
    Dockable? See questions below <<< nobody wants it

Open questions

There are still some open questions where I'd like some feedback:

1 Should the dialog be also available as a dockable panel? If yes, what's the best UX pattern to let the users choose what behavior they want (a global setting, a GUI checkbox inside the dialog, a push button ...) ? <<< nobody wants it
2. I would like to keep the current layers toolbar available (maybe disabled by default) power users may need it, in this case the unified button should probably stay in its own toolbar to allow for customization
3. For the same reason I'd like to keep the menu entries and their shortcuts.
4. for both 2 and 3, we can either keep the current behavior (they open the individual provider dialog) or turn it into opening the unified dialog wtith the correct "tab" open

Looking forward to hear your feedback!

@jonnyforestGIS

This comment has been minimized.

Copy link
Contributor

commented May 24, 2017

Hi @elpaso ,
great 👍

My opinion:

In question 1, perhaps put as an option checkbox in settings> tab data sources like the open attribute table in a dock windows?

I agree with you in question 2 and 3, for example sometimes i need to customize the GUI for non-gis users.

For question 4, if the setting checkbox (option checkbox that i refer in question 1) is active will show the unified layers if not keep the individual provider dialog.

Cheers
João

@timlinux

This comment has been minimized.

Copy link
Member

commented May 24, 2017

@elpaso thanks so much for making this! Its looking really good!

There are still some open questions where I'd like some feedback:

Should the dialog be also available as a dockable panel? If yes, what's the best UX pattern to let the users choose what behavior they want (a global setting, a GUI checkbox inside the dialog, a push button ...) ?

I'd be -1 for that - the dialog isn't going to fit well in dock and we already have the browser for quickly dropping a good proportion of data sources into the canvas so it seems redundant to make this a dock widget too...

I would like to keep the current layers toolbar available (maybe disabled by default) power users may need it, in this case the unified button should probably stay in its own toolbar to allow for customization

+1 from me to make it available still as an option (default hidden)

For the same reason I'd like to keep the menu entries and their shortcuts.
for both 2 and 3, we can either keep the current behavior (they open the individual provider dialog) or turn it into opening the unified dialog wtith the correct "tab" open

+1 from me

Thanks!

class QgsDataSourceManagerDialog;
}

class QgsDataSourceManagerDialog : public QgsOptionsDialogBase

This comment has been minimized.

Copy link
@nyalldawson

nyalldawson May 25, 2017

Contributor

Can we move this to GUI? I think there's a strong case for allowing plugins to show a generic "select ANY available vector/raster" layer dialog

This comment has been minimized.

Copy link
@elpaso

elpaso May 25, 2017

Author Contributor

@nyalldawson I'm trying to move it to gui but the dialog needs the browser panel which is in app, what would you recommend to do?

This comment has been minimized.

Copy link
@nyalldawson

nyalldawson May 30, 2017

Contributor

I'd suggest moving that widget across to gui too - I think it'd be handy to have accessible to plugins too.

This comment has been minimized.

Copy link
@elpaso

elpaso May 30, 2017

Author Contributor

I agree. But this is out of scope for this PR.
The road to a refactoring of the provider dialogs is paved with obstacles.
BTW I'll see if I can get some funds for this in a future iteration.

This comment has been minimized.

Copy link
@elpaso

elpaso May 31, 2017

Author Contributor

@wonder-sk I'm looking at the move of QgsBrowserDockWidget to gui that is mandatory if we want to move this whole layer dialog to gui, but I see that due to the delayed model initialization, the app is needed. Can you suggest what to do here? I'd probably add the app as an optional param to the class ctor, but you might have a better idea for this decoupling.
See: https://github.com/qgis/QGIS/blob/master/src/app/qgsbrowserdockwidget.cpp#L341
As an alternative, we could emit a signal when the model is created and make the connection from the app. What do you think?
PS:in any event this would be part of a separate PR.

This comment has been minimized.

Copy link
@manisandro

manisandro Jun 14, 2017

Member

@nyalldawson I'll take a look!

This comment has been minimized.

Copy link
@manisandro

manisandro Jun 15, 2017

Member

@nyalldawson Looks like the solution is to introduce a shared struct analogous to QgsWFSSharedData. I'll work on it.

This comment has been minimized.

Copy link
@nyalldawson

nyalldawson Jun 15, 2017

Contributor

Sounds perfect - thanks!

This comment has been minimized.

Copy link
@elpaso

elpaso Jun 23, 2017

Author Contributor

@nyalldawson @wonder-sk I'm looking at moving this class to GUI, but I've found more dependencies, like qgsnewogrconnection.h and all its companions under app/ogr, do you think that worths the effort moving all ogr deps to GUI too? I start to doubt about that.

This comment has been minimized.

Copy link
@elpaso

elpaso Jun 23, 2017

Author Contributor

@nyalldawson moved to GUI in #4769

@@ -98,7 +98,7 @@ class CORE_EXPORT QgsProviderRegistry
* responsible for deleting the returned widget.
*/
QWidget *createSelectionWidget( const QString &providerKey,
QWidget *parent = nullptr, Qt::WindowFlags fl = Qt::WindowFlags() );
QWidget *parent = nullptr, Qt::WindowFlags fl = Qt::WindowFlags(), bool embeddedMode = false );

This comment has been minimized.

Copy link
@nyalldawson

nyalldawson May 25, 2017

Contributor

instead of a bool here, I'd rather see a "widget mode" enum used

This comment has been minimized.

Copy link
@elpaso

elpaso May 25, 2017

Author Contributor

Done in 8564e8b

connect( ovl, &QgsOpenVectorLayerDialog::addVectorLayers, this, &QgsDataSourceManagerDialog::vectorLayersAdded );

// Add data provider dialogs
QDialog *dlg;

This comment has been minimized.

Copy link
@nyalldawson

nyalldawson May 25, 2017

Contributor

uninitialized

// Add data provider dialogs
QDialog *dlg;

#ifdef HAVE_POSTGRESQL

This comment has been minimized.

Copy link
@nyalldawson

nyalldawson May 25, 2017

Contributor

Could we avoid all this hardcoding and just get the providers direct from the registry?

This comment has been minimized.

Copy link
@elpaso

elpaso May 25, 2017

Author Contributor

I'll have a second look but I'm afraid that's not currently possible, the different providers dialogs have a (slightly) different interface or need a different setup: see for example AMS that needs the canvas extent, and some (but not all) of the DB that have a progress dialog, it's bit messy and this also answers to the unbound signals reported by @nirvn (I'll try to address that too btw, I tried to use a common function for all DB-like providers, but some of those lack the progress dialog).

@nyalldawson

This comment has been minimized.

Copy link
Contributor

commented May 25, 2017

Really nice - I gave it a test drive and it's certainly a big improvement! well done!

@guestagain

This comment has been minimized.

Copy link

commented May 25, 2017

Hello Folks, I have an additional suggestion to this feature which may or may not be possible or desired by anyone else but here it goes (sorry do not have the skills to assist with development):

The idea is that in the ~"add any layer" dialog that is been developed here, to the right hand side of that dialog there is a check box that you can click on (default = off) that allows you to open the selected layer as a copy of the file you have selected to open, saving it either in the same folder that it is sourced from - by clicking on an additional check box labelled ~"save in same folder with for example '_001' appended to the end of the original filename", or; with a third column with the usual box with the three little dots that allow you to browse to a selected location and set the desired new filename.

If this is not clear I have uploaded two very (very) rough screenshots of how the implementation would look at the following addresses (you may at least have a giggle at the magnificent msPaint design skills on show):

  1. http://imgur.com/7r1FlMq
  2. http://imgur.com/6CicFdN

[note: these are annotations on two alternative versions of this ~"add any layer" concept found at these two web locations:

  1. #4629 (here) and;
  2. https://anitagraser.com/2016/07/20/one-add-button-to-rule-them-all/

Aside: a feature like this, as well as an extra check box/directory search that allows you to set the .qml file prior to opening would be great also.

If it is not possible to implement as part of the dialog box been discussed here, perhaps it may be useful for consideration in the usual "add shapefile" dialogs within QGIS.

Good luck and thank you for all your amazing work in any case.

@nirvn

This comment has been minimized.

Copy link
Contributor

commented May 25, 2017

I like how this is being implemented, good job.

One thing this new dialog has revealed is the UI inconsistencies between various server provider connection dialogs:

  • "Connections" (e.g. PostgreSQL) vs. "Server Connections" (WFS) vs no title (WCS)
@nirvn

This comment has been minimized.

Copy link
Contributor

commented May 25, 2017

BTW, console had a couple of warnings popping up:

Warning: QObject::connect: No such signal QgsSpatiaLiteSourceSelect::progress( int, int )
Warning: QObject::connect:  (sender name:   'QgsDbSourceSelectBase')
Warning: QObject::connect:  (receiver name: 'QgsDataSourceManagerDialog')
Warning: QObject::connect: No such signal QgsSpatiaLiteSourceSelect::progressMessage( QString )
Warning: QObject::connect:  (sender name:   'QgsDbSourceSelectBase')
Warning: QObject::connect:  (receiver name: 'QgsDataSourceManagerDialog')
Warning: QObject::connect: No such signal QgsMssqlSourceSelect::progress( int, int )
Warning: QObject::connect:  (sender name:   'QgsDbSourceSelectBase')
Warning: QObject::connect:  (receiver name: 'QgsDataSourceManagerDialog')
Warning: QObject::connect: No such signal QgsMssqlSourceSelect::progressMessage( QString )
Warning: QObject::connect:  (sender name:   'QgsDbSourceSelectBase')
Warning: QObject::connect:  (receiver name: 'QgsDataSourceManagerDialog')
Warning: QObject::connect: No such signal QgsDb2SourceSelect::progress( int, int )
Warning: QObject::connect:  (sender name:   'QgsDbSourceSelectBase')
Warning: QObject::connect:  (receiver name: 'QgsDataSourceManagerDialog')
Warning: QObject::connect: No such signal QgsDb2SourceSelect::progressMessage( QString )
Warning: QObject::connect:  (sender name:   'QgsDbSourceSelectBase')
Warning: QObject::connect:  (receiver name: 'QgsDataSourceManagerDialog')
Warning: QObject::connect: No such signal QgsAmsSourceSelect::addRasterLayer( QString const &, QString const &, QString const & )
Warning: QObject::connect:  (sender name:   'QgsSourceSelectBase')
Warning: QObject::connect:  (receiver name: 'QgsDataSourceManagerDialog')
Warning: QMetaObject::connectSlotsByName: No matching signal for on_mDialogButtonBox_helpRequested()
@nirvn

This comment has been minimized.

Copy link
Contributor

commented May 25, 2017

I'd recommend removing the close button visible in the GIF shot. It saves space, and fits other parts of QGIS (snapping configuration, style manager)

@nirvn

This comment has been minimized.

Copy link
Contributor

commented May 25, 2017

Also, I'd put the [+] button as part of the layers panel toolbar. IMHO it's a better suited and more natural placement for it, and it'll allow ppl to hide the main toolbar with individual providers off if not needed.

@elpaso

This comment has been minimized.

Copy link
Contributor Author

commented May 25, 2017

@guestagain thanks for your ideas, this PR is limited in scope to the first phase as described at the end of the linked google doc: embed the browser and the current provider dialogs into a single one.

The second phase will target the browser to make it feature complete (features parity with the provider dialogs).

So, please add your ideas to the google document, or file a feature request on the hub.

@elpaso

This comment has been minimized.

Copy link
Contributor Author

commented May 25, 2017

@nirvn

Also, I'd put the [+] button as part of the layers panel toolbar. IMHO it's a better suited and more natural placement for it, and it'll allow ppl to hide the main toolbar with individual providers off if not needed.

I'm sorry but I don't understand what you mean, the "+" button is currently already part of the layer toolbar:

immagine

But, because the actual buttons for the dialogs will be optional (probably they will just stay where they are and the layer toolbar will be hidden by default) I believe we need to put the "+" button in a separate toolbar.

Suggestions welcome!

@elpaso

This comment has been minimized.

Copy link
Contributor Author

commented May 25, 2017

@nirvn

I'd recommend removing the close button visible in the GIF shot. It saves space, and fits other parts of QGIS (snapping configuration, style manager)

I see your point but the close button is actually there for consistency with the other dialogs (options, layers properties, about, plugin manager etc.), in fact it is required by the base class QgsOptionsDialogBase (and I'm going to add an Help button too).

I'm not convinced that from a usability perspective, removing the Close button is a good idea, but in case we take that route, that requirement should be removed from the base class so that we have a consistent GUI.

BTW, your comment brings to the front another important topic: we should write a more comprehensive HIG document.

@nirvn

This comment has been minimized.

Copy link
Contributor

commented May 25, 2017

@elpaso , here's what I meant, in image :)
screenshot from 2017-05-25 14-21-32

@elpaso

This comment has been minimized.

Copy link
Contributor Author

commented May 25, 2017

@nirvn oh, I see, thanks for the snapshot :)

That makes sense, I'm just worried because that it's a PANEL and could be hidden, but that would be quite an odd setup anyway.

@nirvn

This comment has been minimized.

Copy link
Contributor

commented May 25, 2017

@elpaso , in terms of panel hidden, we could have the same argument with the layer management toolbar (i.e., user turning it off). I think in this case, the panel is a better placement 😄

@SrNetoChan

This comment has been minimized.

Copy link
Member

commented May 25, 2017

I like @nirvn idea. IMHO, it's a very convenient and also intuitive place to have a button to load data.

About Question 2: I would really like if this could be a dockable panel, hence non-modal dialog. Those with enough screen space could use it docked, while others could use it in floating mode with the exact same functionality it as now.

Also, I was under the impression that this data manager dialog/panel would replace the browser (at least in the long run), no? Otherwise, we are just adding yet another way to load data, which IMHO, may end up being more confusing, and harder to maintain.

@DelazJ

This comment has been minimized.

Copy link
Contributor

commented May 25, 2017

@SrNetoChan

Also, I was under the impression that this data manager dialog/panel would replace the browser (at least in the long run), no? Otherwise, we are just adding yet another way to load data, which IMHO, may end up being more confusing, and harder to maintain.

This is what I thought too: add to the Browser Panel in a more convenient way the missing features from all the other data loader. And I do not feel it's the case reading some comments and due to the screencast.
Nota: I didn't test the branch yet hence might be missing something

@elpaso

This comment has been minimized.

Copy link
Contributor Author

commented May 25, 2017

@SrNetoChan , @DelazJ

It should be all in the linked Google doc.

The roadmap is to eventually get rid of the dialogs and keep the browser. But this is not in scope for this PR because the browser is not feature complete.

Phase 1 (current PR) is to hide the individual providers dialogs and toolbar and replace them with this unified dialog with the embedded browser as the default tab.

Phase 2 (future PR, to be assigned) is to add the missing features to the browser and be able to do every single operation from it.

So, all functionality currently missing from the browser will be temporarily available in the dialogs.

and harder to maintain.

not as long as it uses the browser as an embedded widget (they share the same code).

@nirvn

This comment has been minimized.

Copy link
Contributor

commented May 25, 2017

@elpaso , I'm -1 on removing the toolbar. One reason why I think the [+] button should be located in the layer panel is to actually preserve the provider toolbar and be able to have these useful shortcuts at hand. Clicking on the individual provider toolbar icon either opens the individual provider dialog, or opens the unified dialog focused on that specific provider's tab (no strong feeling either way here).

@wonder-sk

This comment has been minimized.

Copy link
Member

commented May 25, 2017

@elpaso very nice!

Would it be possible to add support for an interface so that plugins can register custom pages for "add layer" dialog? This would be also useful to make the existing widgets more uniform as they would be all using a common interface...

@elpaso elpaso force-pushed the boundlessgeo:unified-button branch from cb55cf7 to 8564e8b May 25, 2017

@elpaso

This comment has been minimized.

Copy link
Contributor Author

commented May 26, 2017

@wonder-sk yes, that would be a nice addition, thanks for raising the point.

Unfortunately there is no common interface for the Qgs<name your provider>SourceSelect dialogs, and some even have post configuration. An additional blocker is the complete lack of homogeneous signals and slot naming in the providers (for instance we have addVectorLayer but also addWfsLayer etc.).

I'm afraid that this refactoring would be out of scope for this PR, but I totally agree with you that we need to eventually address a deeper refactoring of the providers and related dialogs towards a common interface,

BTW, with this PR I've already started to make the function signatures for the dialogs factory look the same for all providers, at least it is a step in the right direction, see: 8564e8b

@SrNetoChan

This comment has been minimized.

Copy link
Member

commented May 26, 2017

We have been discussing some aspects of the UI/UX of this new functionality. To allow @elpaso complete this work, we need to make some decisions. Note that the create new layer buttons should not be included in the data source manager, so we will have two buttons to manage. So, here it goes a summary of what we have until now:

Kinda consensual?:
1 - Old data management toolbar should not be removed, but only disabled by default;
2 - Menu entries for each provider should not me removed either (to keep shortcuts).

3 - Where to put the "unified button" and create new layer button?
a) In a separate toolbar (to enable turning the rest of the buttons on and off)
image
image

b) In the layers panel toolbar.
image
pros: a convenient place to load data
cons: Layer panel can be closed; Too many buttons in the layers panel's toolbar;

4 - If we go with solution 3a, where, by default, should we put the new toolbar?

Note: We are talking about the default, so what matters is the perception new users will have. Others can always customize it the way they like the most.

a) Left vertical toolbar area (where it is now)

image

maybe with some company

image

b) Somewhere in the top toolbars (so it does not sit alone in the left, wasting space).

image

@SrNetoChan

This comment has been minimized.

Copy link
Member

commented May 26, 2017

Regarding the menu items..

5 - where should we put the new Add Layer button?
a) In layer > add layer > Open Data source manager.
image

b) In layer > Open Data Source Manager.
image
c) Don't put it on the menus. (may be related to question 2)

7 - Clicking the menu items, what should happen?
a) Open the current provider individual dialogs.
b) Open the new Data Source Manager dialog in the respective provider's tab.
(This would replace the need to have a menu entry for opening the data source manager)

@DelazJ

This comment has been minimized.

Copy link
Contributor

commented May 26, 2017

Imho, the 3.0 changelog should state that there's now only one place to add layers to QGIS. I mean one dialog from the User perspective (whether it's not yet the case in the code base is another issue). So whatever button or menu the user selects, it would open the Data source manager dialog, either on the last tab (using the new entry) or the selected provider tab (using provider's entry).
For the User, the individual modal dialogs should no longer exist. This is a more coherent behavior and might reduce the gap once all this is merged with the Browser panel in the step 2 of the implementation. Also, if we keep showing old dialogs, it will rather looks like we "simply" add another tool instead of reducing the list.
This is the way I conceive these changes.

Trying now to give my opinions for the points @SrNetoChan raised
3a/ yes for a separate toolbar (not add it to current Layers toolbar
b/ I think this placement has a lot of sense. There's a remove layer button so let's add a "Add layer" one. Not sure that the "create layer" button should also go to Layers panel. The fact that the layers panel could be hidden is not really a big argument. Any panel or toolbar in QGIS can be hidden.
4/ I would say (a) with company but no strong opinion
5/ I'd say (b) - do not mix it other buttons because of toolbar separation. But I'd move it upper in the dialog (2nd place?)
7/ according to my initial comment, (b).

Just to be sure, at the end of the implementation, once all is done, there will only be one button (to rule them all as Anita said), right? We'll remove all the current provider's buttons? --> using the current provider's buttons or menu entries is just a temporary configuration. Am I right?

@nirvn

This comment has been minimized.

Copy link
Contributor

commented May 29, 2017

@elpaso , @SrNetoChan , OK, lots to catch up to :)

I think we all agree on point 1 and 2, we can look that in.

Point 3, I'm honestly a -1 to create a two-buttons main window toolbar. IMHO, that's just messy, I'd rather avoid adding one more toolbar to our 14 preexisting ones :) Like I've stated above, in my view, the cleanest and logical approach is to add it to the layer panel, where we already have a remove layer button.

That said, you've raised a good point re "too many buttons" in the layer panel's toolbar. It was already an issue before, and it would be could to fix this now that we're on the topic. Two things should be done to improve the situation: a/ move the "expand all", "collapse all" buttons into the [ eye ] dropdown toolbar button (it'll sit well with the show all / hide all layers, etc), b/ merge the confusing presence of two filter buttons into one dropdown toolbar button.

Point 5, what about putting the open data source manager to be at the very top of the layer menu? You'd want that the closest to your mouse pointer when expanding that menu.

BTW, If you guys don't have the time / resource to come up with an icon for this feature, I can commit one once this PR is merged. Sharing the burden 😉

Point 7, I'd pick option b, namely the menu actions open the data source manager with a focus on the clicked provider tab. I do not see any point in keeping individual dialogs around when we have something that's superior and doesn't get in the way. That'll streamline the UX.

@shenriod

This comment has been minimized.

Copy link

commented May 29, 2017

2 cents: I am much more in favor of 3a) than 3b). Beginners could very easily close the layers panel by mistake and would immediately feel lost "how can I add a new layer?" I see it much less likely that they disable the toolbar by mistake.

However I don't think that both are mutually exclusive. Any strong reason why we could not have 3a) AND 3b) ? I don't see this as a bad redundancy

I would also favor 4b) over 4a)

For 5, I agree with @nirvn : "what about putting the open data source manager to be at the very top of the layer menu? You'd want that the closest to your mouse pointer when expanding that menu."

elpaso added some commits May 25, 2017

[addlayerbutton] Virtual Layers support added
Some changes in connections are required to keep in sync
the available layers lists in the virtual layer dialog.

(Modeless support is on the way)
[addlayerbutton] Raster support added
There is still an issue with the list not being
correclty reset after the raster icon is selected.
[addbuttonlayer] Fix layout of vector dialog
This is an attempt to fix the layout of the vector
layer dialog to fit in the unified dialog layout.

The optiomal solution would probably involve
a complete refactoring of the vector layer dialog
to use a stacked widget.
[addlayerbutton] Removed help button
Almost all provider dialogs have its own button
resulting in duplicated help buttons in the
unified dialog.
[addlayerbutton] Add method to open a given page in the layer dlg
This is preliminary to change all dialogs slots to
open the unified dialog with the righ page open.
[addlayerbutton] Fix order of signals when open page called
The problem was with raster "tab" (that is not a real page
but a forwarded signal to the app), when openPage
was called before the show() call.

Moving the activation inside the unique currentPage call,
the implementation is much simpler (got rid of the lambda
with the second currentRowChanged connection).
[addlayerbutton] Data Source Manager toolbar and menu
This implements the proposal from @srneto:

- new toolbar with the new dialog button and
  all the "new" data soource buttons
- old layer toolbar hidden by default
- new dialog open from first item in layer menu
[addlayerbutton] Removed button bar from main dialog
No Close button anymore ... prepare your wrist for
a precise hit on the [X] or hit ESC to close the
dialog.
[addlayerbutton] Fix height problems within the delimited text dialog
Also moved the status information under the button bas for
consistency with other dialogs.

@elpaso elpaso force-pushed the boundlessgeo:unified-button branch from d8ecb49 to c188911 Jun 2, 2017

@elpaso elpaso merged commit 1067239 into qgis:master Jun 2, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@elpaso elpaso deleted the boundlessgeo:unified-button branch Jun 3, 2017

@shenriod

This comment has been minimized.

Copy link

commented Jun 6, 2017

Thanks all who contributed to this new feature!

A few comments and requests for opinions after a few tests:

Behavior of the Raster provider
I agree with @nyalldawson and @nirvn that the raster provider should not open directly the browser. A very simple UI (only with the browse button?) would probably be more intuitive for the users

DB2 Icon with non-transparent background
I checked the svg here https://github.com/qgis/QGIS/blob/master/images/themes/default/mIconDb2.svg and here https://github.com/qgis/QGIS/blob/master/images/themes/default/mActionAddDb2Layer.svg and they seem to have a transparent background. Thus not sure why it's not being rendered correctly?

Order of the icons
I would suggest a re-ordering of the icons (and slight renaming), so that all file-based types are together, all db-based together, etc...

What about:

  • Browser
    [separation line]
  • Vector (not Vector files)
  • Raster
  • Delimited Text
    [separation line]
  • PostgreSQL
  • Spatialite
  • MSSQL
  • DB2
    [separation line]
  • WMS
  • WFS
  • WCS
  • ArcGIS Map Server
  • ArcGIS Feature Server
    [separation line]
  • Virtual layer (not Virtual)

Any opinion / objection on this?

Cheers!

@DelazJ DelazJ referenced this pull request Jun 12, 2017

Merged

Reorganize the Datasource Manager tabs #4716

3 of 8 tasks complete
@nirvn

This comment has been minimized.

Copy link
Contributor

commented Jun 23, 2017

@elpaso , after giving this nice UX upgrade a test run for a week now, I really think the raster provider needs to have a simple [ browse ] UI ala OGR vector provider. It's a bit irritating to re-open the add layer button and not have it stick to raster when it's the last used provider.

Plus, pasting an URL for a raster (i.e. /home/gisuser/rasters/my.tif) in a text box can be quicker than using the file picker dialog 😄

@elpaso

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2017

@nirvn no objections from my side.

@DelazJ

This comment has been minimized.

Copy link
Contributor

commented Jun 23, 2017

From my side, something that puzzles me is that one of the main target of this restructuring (or at the end of the whole process) is to give more visibility to the QGIS Browser. And while experimenting the Data source manager dialog, I almost never use the embedded browser. None of the existing icons opens that tab. In other words, i'm afraid the new dialog is hiding the qgis browser...
How about keep opening the corresponding tab when one of the usual tools is used and opening the browser tab when someone hits the add layer button itself (instead of the latest opened tab) ? Not sure though it'd be a nice UX. But it could also be a temporary state (until the total revamp).

@luipir

This comment has been minimized.

Copy link
Contributor

commented Jun 23, 2017

@DelazJ Hide Browser? You proposal is to go back to Browser everytime? Can be a simply a configurable behavior (previous tab or tab as default and by default Browser tab) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.