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

bad layers dialog new button labels and icon #8435

Merged
merged 5 commits into from
Nov 8, 2018

Conversation

elpaso
Copy link
Contributor

@elpaso elpaso commented Nov 7, 2018

Followup #8359 (comment)

immagine

@SrNetoChan @nirvn what do you think?

@m-kuhn
Copy link
Member

m-kuhn commented Nov 7, 2018

Proposal sed -i 's/Bad/Unavailable/g

@elpaso

This comment has been minimized.

@nirvn
Copy link
Contributor

nirvn commented Nov 8, 2018

Ok, I think we need to insure browse and apply are paired together (separating them like the shot above is counterintuitive

My suggestion would be to move browse and apply to be left end and have the remove & keep buttons at the right end.

@nirvn
Copy link
Contributor

nirvn commented Nov 8, 2018

@elpaso , how are things look like now?

@elpaso
Copy link
Contributor Author

elpaso commented Nov 8, 2018

Looks good, see the new picture

@elpaso
Copy link
Contributor Author

elpaso commented Nov 8, 2018

I just want to add a small fix on "Change data source..." and disable it when layer is editable, then I'll merge

@nirvn
Copy link
Contributor

nirvn commented Nov 8, 2018

@elpaso , nice, undeniable improvement.

@nirvn
Copy link
Contributor

nirvn commented Nov 8, 2018

@elpaso , any chance you could tweak the layer menu order:
image

IMHO, open attribute table should be a the top of the menu section, followed by toggle editing. I don't have strong feelings as to whether filter or change data source should come first.

@DelazJ
Copy link
Contributor

DelazJ commented Nov 8, 2018

I don't have strong feelings as to whether filter or change data source should come first.

I would say Filter... before Change Data Source... (btw, missing title case).

@nirvn
Copy link
Contributor

nirvn commented Nov 8, 2018

@DelazJ , yeah, that's where my vote would go on the ballot too.

@elpaso
Copy link
Contributor Author

elpaso commented Nov 8, 2018

Anybody can provide a nice icon for that action?

@nirvn
Copy link
Contributor

nirvn commented Nov 8, 2018

@elpaso , IMHO, it's a good candidate for an icon-less action.

@@ -200,15 +200,23 @@ QMenu *QgsAppLayerTreeViewMenuProvider::createContextMenu()
menu->addSeparator();

// change data source is only supported for vectors and rasters
if ( vlayer || rlayer )
if ( ( vlayer || rlayer ) )
Copy link
Member

Choose a reason for hiding this comment

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

Are these extra parentheses on purpose?

menu->addAction( a );
// Disable when layer is editable
Copy link
Member

Choose a reason for hiding this comment

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

If this comment needs to stay I think it should go to line 208

@elpaso elpaso merged commit eab40d0 into qgis:master Nov 8, 2018
@elpaso elpaso deleted the handle-bad-layers5 branch November 8, 2018 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants