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

[FEATURE] Layer tree view indicator for non-removable (required) layers #7862

Merged
merged 4 commits into from
Sep 11, 2018

Conversation

wonder-sk
Copy link
Member

It makes it easier for the user understand that the layers are marked as such in project properties.

image

I am thinking of adding a parent class for layer-based indicators to lower the amount of duplicate code. Or maybe even merge all layer-based indicator providers into one? Not sure.

cc @ghtmtt

@nirvn
Copy link
Contributor

nirvn commented Sep 11, 2018

Style-wise, the icon should be outline-only.

Other than that, cool :)

@nyalldawson
Copy link
Collaborator

There is a lot of mostly duplicate code going on here. Reducing this would be good if it's possible!

One thing I noticed lately too which would be good to address is that indicators stop working if you move a layer into a different group.

mFlags = flags;
emit flagsChanged();
Copy link
Member

Choose a reason for hiding this comment

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

flags are also changed in readLayerStyle at line 418 and 420.
I was a bit lazy....let's call setFlags.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in 44611d7

@wonder-sk
Copy link
Member Author

@nirvn is the new icon better? Sorry I'm not a graphic designer :-)

image

@wonder-sk
Copy link
Member Author

There is a lot of mostly duplicate code going on here. Reducing this would be good if it's possible!

I would like to do it after the feature freeze if fellow devs do not object - there are few more bits on my plate until Friday :-)

One thing I noticed lately too which would be good to address is that indicators stop working if you move a layer into a different group.

@nyalldawson I was told this before but I have never seen it myself :-( Is replicating as simple as starting a project with one memory layer (which shows an indicator), add a group, move the layer there - and the indicator is gone? Because that works just fine for me...

@nyalldawson
Copy link
Collaborator

Because that works just fine for me...

Maybe it's in nested groups. Let me check

@nyalldawson
Copy link
Collaborator

I would like to do it after the feature freeze if fellow devs do not object - there are few more bits on my plate until Friday :-)

Fine with me!

@nyalldawson
Copy link
Collaborator

Ok try this

  1. Start a new project
  2. Add a layer
  3. Create a group, move the layer to that group
  4. Add a filter to the layer = no indicator
  5. Move layer out of group = indicator appears

@wonder-sk
Copy link
Member Author

Thanks Nyall - I could finally replicate the issue - it is related to drag'n'drop of the layer and the fact that we disconnect from the signal even though the layer is still present. Will fix when doing the code de-duplication.

@nirvn
Copy link
Contributor

nirvn commented Sep 11, 2018

@wonder-sk , looking much better. To be consistent, we'd need a 1px light gray shadow of the outline. I can easily add that to the new file you've created after you merged this if that helps.

@wonder-sk
Copy link
Member Author

I can easily add that to the new file you've created after you merged this if that helps.

Thanks!

@wonder-sk wonder-sk merged commit 318946a into qgis:master Sep 11, 2018
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.

4 participants