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.

Sign up for GitHub

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

Jump to bottom

Misleading lock symbol icon in the in the Data tab toolbar #870

Merged
merged 5 commits into from
Aug 10, 2020
Merged

Misleading lock symbol icon in the in the Data tab toolbar #870

merged 5 commits into from
Aug 10, 2020

Conversation

lindakarlovska
Copy link
Contributor

@lindakarlovska lindakarlovska commented Aug 2, 2020 •
edited
Loading

Describe the bug

The toolbar of Data tab uses lock/unlock symbol for allowing and disallowing edits in mapsets other than the current one. However, the locking refers to something else in GRASS GIS and that it locking of mapsets by a GRASS GIS process/session. That terminology is actually in line with other software where a file is locked when used by another process or instance of the same program.

Expected behavior

A new icon is needed. It may have the allowed and restricted states like now or it could be just one symbol relying on the toggle state of the button to convey the change of state. A common icon for editing in GIS, including GRASS GIS, is a pen/pencil, so that's at least a good starting point.

Suggestion:

Editing mode allowed:

allow_edits

No edits allowed:

disallow_edits

What are your suggestions?

wxGUI/datacatalog: Created two icons for allowing and disallowing edi…
dfb1a55 
…ting mode. The red icon is taken from QGIS mActionCancelAllEdits.svg, green icon was created.
@lindakarlovska lindakarlovska marked this pull request as draft August 2, 2020 09:02
@mlennert
Copy link
Contributor

mlennert commented Aug 3, 2020

Describe the bug

The toolbar of Data tab uses lock/unlock symbol for allowing and disallowing edits in mapsets other than the current one. However, the locking refers to something else in GRASS GIS and that it locking of mapsets by a GRASS GIS process/session. That terminology is actually in line with other software where a file is locked when used by another process or instance of the same program.

Expected behavior

A new icon is needed. It may have the allowed and restricted states like now or it could be just one symbol relying on the toggle state of the button to convey the change of state. A common icon for editing in GIS, including GRASS GIS, is a pen/pencil, so that's at least a good starting point.

Suggestion:

Editing mode allowed:

allow_edits

No edits allowed:

disallow_edits

What are your suggestions?

I don't have any strong feelings, but if forced to choose,I probably would prefer the suggestion of a simple pencil showing whether the button is pushed or not, instead of a change of icon.

@lindakarlovska
Copy link
Contributor Author

Describe the bug
The toolbar of Data tab uses lock/unlock symbol for allowing and disallowing edits in mapsets other than the current one. However, the locking refers to something else in GRASS GIS and that it locking of mapsets by a GRASS GIS process/session. That terminology is actually in line with other software where a file is locked when used by another process or instance of the same program.
Expected behavior
A new icon is needed. It may have the allowed and restricted states like now or it could be just one symbol relying on the toggle state of the button to convey the change of state. A common icon for editing in GIS, including GRASS GIS, is a pen/pencil, so that's at least a good starting point.
Suggestion:
Editing mode allowed:
allow_edits
No edits allowed:
disallow_edits
What are your suggestions?

I don't have any strong feelings, but if forced to choose,I probably would prefer the suggestion of a simple pencil showing whether the button is pushed or not, instead of a change of icon.

Do you think something like that?
No edits allowed:
disallow_edits2

Editing mode allowed:
allow_edits2
allow_edits3

@mlennert
Copy link
Contributor

mlennert commented Aug 4, 2020

Yes, or the editing mode button in QGIS.

But I'm definitely neither a UX expert, nor up to date in modern design, so I might not be the best to ask. You could send a mail to the dev and/or user list and ask...

@neteler
Copy link
Member

neteler commented Aug 4, 2020

It might be worth checking if the red/green colors are colorblind safe.

@petrasovaa
Copy link
Contributor

Yes, or the editing mode button in QGIS.

I agree, we already that editing icon available. Probably we don't need a special icon for toggled state. Let's keep it simple and see later if people will find it misleading.

@lindakarlovska lindakarlovska added GUI wxGUI related bug Something isn't working question Further information is requested labels Aug 5, 2020
@lindakarlovska lindakarlovska self-assigned this Aug 5, 2020
@lindakarlovska lindakarlovska added the gsoc Reserved for Google Summer of Code student(s) label Aug 5, 2020
@lindakarlovska
Copy link
Contributor Author

lindakarlovska commented Aug 5, 2020 •
edited
Loading

Yes, or the editing mode button in QGIS.

I agree, we already that editing icon available. Probably we don't need a special icon for toggled state. Let's keep it simple and see later if people will find it misleading.

I used this icon mActionToggleEditing.svg available in QGIS:
toggleEditing

@petrasovaa
Copy link
Contributor

Yes, or the editing mode button in QGIS.

I agree, we already that editing icon available. Probably we don't need a special icon for toggled state. Let's keep it simple and see later if people will find it misleading.

I used this icon mActionToggleEditing.svg available in QGIS:
toggleEditing

We already have the edit.png in grass, so just add the svg to have both, I think they match.

@petrasovaa
Copy link
Contributor

This works. One thing we probably should do is to keep the changing tooltip depending on the state (the icon would be the same).
The tooltip itself may need to be changed but I am not sure how. It says "Allow editing other mapsets", do users know what we mean with other mapsets? Should we add "and locations"? Or we could say "Do not restrict edits to current mapset only" and "Restrict edits to current mapset only". Or something else, any suggestions @mlennert, @wenzeslaus, @lindakladivova ?

@mlennert
Copy link
Contributor

mlennert commented Aug 7, 2020 via email

@petrasovaa petrasovaa added this to PRs in progress in New GUI Startup via automation Aug 7, 2020
@petrasovaa petrasovaa linked an issue Aug 7, 2020 that may be closed by this pull request
[Bug] Misleading lock symbol icon in the in the Data tab toolbar #844
Closed
@lindakarlovska
Copy link
Contributor Author

lindakarlovska commented Aug 7, 2020 •
edited
Loading

Am 6. August 2020 22:36:49 MESZ schrieb Anna Petrasova notifications@github.com:
This works. One thing we probably should do is to keep the changing tooltip depending on the state (the icon would be the same). The tooltip itself may need to be changed but I am not sure how. It says "Allow editing other mapsets", do users know what we mean with other mapsets? Should we add "and locations"? Or we could say "Do not restrict edits to current mapset only" and "Restrict edits to current mapset only". Or something else, any suggestions @mlennert, @wenzeslaus, @lindakladivova ?
I think "Allow editing other mapsets and locations" is the clearest.

I like "Allow editing other mapsets and locations" as well.

@lindakarlovska lindakarlovska marked this pull request as ready for review August 7, 2020 12:36
petrasovaa
petrasovaa requested changes Aug 8, 2020
View reviewed changes
Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

You still need to change this to show different tooltip for each state. Basically you leave there the original code but use the same icon.

@wenzeslaus
Copy link
Member

The current mapset will be something explicitly mentioned in the tree after #849 is in, so I think centering the message around current mapset would be good. Here are the two tooltips for the two states. Both are formulated without using a negative and the first one is also trying for a positive tone.

  • Allow edits outside of the current mapset
  • Restrict edits to the current mapset only

@lindakarlovska
Copy link
Contributor Author

The current mapset will be something explicitly mentioned in the tree after #849 is in, so I think centering the message around current mapset would be good. Here are the two tooltips for the two states. Both are formulated without using a negative and the first one is also trying for a positive tone.

* Allow edits outside of the current mapset

* Restrict edits to the current mapset only

Ready for review.

@petrasovaa petrasovaa merged commit 8663ae5 into OSGeo:master Aug 10, 2020
New GUI Startup automation moved this from PRs in progress to Done Aug 10, 2020
@neteler neteler added this to the 8.0.0 milestone Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrasovaa petrasovaa petrasovaa approved these changes

@mlennert mlennert mlennert approved these changes

Assignees

@lindakarlovska lindakarlovska

Labels
bug Something isn't working gsoc Reserved for Google Summer of Code student(s) GUI wxGUI related question Further information is requested
Projects
No open projects
New GUI Startup
  
Done
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

[Bug] Misleading lock symbol icon in the in the Data tab toolbar
5 participants
@lindakarlovska @mlennert @neteler @petrasovaa @wenzeslaus