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 modify and remove vector tools #101

Merged
merged 5 commits into from
May 31, 2017

Conversation

theduckylittle
Copy link
Member

  1. Modify now works on selection shapes and other vector layers.
  2. Remove is now available for vector layers.

A modify geometry action was added in order to ensure the store was properly updated and not just the openlayer's layer after a feature was finished.

@klassenjs
Copy link
Member

I don't know what we have for options or how others view it, but the rubber eraser doesn't convey 'Remove All' to me. If anything, the trash can conveys more 'Remove All' (throw out the whole sheet) while the rubber eraser conveys more rub-out an item. @theduckylittle, @elil, @CaitW, others, thoughts?

It looks good to me otherwise.

@theduckylittle
Copy link
Member Author

I was flailing a bit there. I guess for me I have the verbs "remove" and "clear." So "remove" is delete and I think of the trash can. Clear has less meaning (and maybe limited usefulness?) so "Clearing" means "erasing" and the eraser makes sense. This is just how my mind was working when I put the CSS down but there's a lot of room for opinion in there. I even recognize the fallacy of my "clear" association.

Here's a picture of what @klassenjs is talking about:
image

@theduckylittle
Copy link
Member Author

References tickets #55 and #47

@elil
Copy link
Member

elil commented May 30, 2017

This is tricky. I don't really like what is proposed but am slightly hesitant to criticize since I haven't thought of anything better. I'm reviewing some past options to see if it jogs anything.

remove_and_clear_singular_and_plural

remove_feature

remove_all_features

To me the verb is the same (delete, remove, clear, erase, trash, etc). What is different is the direct object or what is being acted upon (one of all).

That is what is clear to me in the previous examples, the difference in singular and plural and the difference in ALL or (ONE). Changing verbs when the action is the same is confusing. The verb changing made the previous versions worse. It is the direct object (or the target of the verb) that is changing.

The way I see this is that we can,
"[verb] (delete, remove, clear, erase, trash, etc) ONE Feature/Drawing"
or we can (exact same verb),
"[verb] (delete, remove, clear, erase, trash, etc) ALL . FeatureS/DrawingS"

Unless the verb indicates a difference in direct object number I don't see the verb as the way to communicate this. (destroy, wipe out, abolish, scorched earth vs decimate, reduce, decrement, etc)

So this is an idea, that we are looking at differentiating the direct object and keeping the verb the same. I still have no ideas on how to communicate that in icons. I feel okay about the word communication above. Maybe a red x over a 1 and a red x over a M (for many - yeah that is stretching it), or maybe a red x for all and a red x over a 1 for one. Or a subtract icon.

@theduckylittle
Copy link
Member Author

Subtract (1) vs Trash Can (all) is something I could +1.

@klassenjs
Copy link
Member

Random thoughts:

Font Awesome has minus-circle which would generally match GM2.x.

While, the use of subtract made sense in GM2 since the draw tools had an (almost too small to see) '+' on them, it always seemed a bit out of place to me as a drawing tool. I personally like the eraser better.

QGIS uses Trash Can to delete selected features (and has only one tool rather than splitting the one and all cases). Of course, we don't really implement the concept of selected features.

GIMP uses an Eraser to remove what is clicked on.

Most other software seems to not have a GUI representation for delete object and relies on a menu item or keypress after the object is selected. The closest is the scissors typically used for cut, but cut implies the object can later be pasted which is not the case for us.

@elil
Copy link
Member

elil commented May 30, 2017

Are you thinking, "-(1)" and "[trash can] (all)"?

What about keeping the verb the same?

"-(1)" and "-(all)" or "[trash can] (1)" and "[trash can] (all)"?

minus

@elil
Copy link
Member

elil commented May 30, 2017

or eraser (1) and eraser (all)?

@klassenjs
Copy link
Member

Semantically I like that. I am not sure how well it would fit and/or translate. Would an icon for the verb and a 1 or * subscript/superscript superimposed on it work? If so do we know how to make it?

My personal preference is eraser for the icon but I wouldn't -1 the others.

@elil
Copy link
Member

elil commented May 30, 2017

I like 1 and all optionally in parenthesis, (1) and (all). Do we need super/sub-script? I guess it can be hard to tell where one icon stops and the next begins without some super/sub-script.

I'm not sure that the * wildcard is going to be readily recognized as all.

I like the eraser but am not opposed to the other icons.

@theduckylittle
Copy link
Member Author

I really don't like having the additional text descriptors and/or super/subscripts for a few reasons:

  1. It makes the CSS patten we've got established a lot trickier.
  2. It makes the layout of a single icon much trickier in the catalog. Particularly once more flexible layouts and smaller screen sizes start getting throw in, we'll have issues relating the text and the icon (overflow spacing is real).

@klassenjs put together another ticket but I'm going to make the following changes:

  1. Match gm2's "-" circle and trash can to distinguish the two tools.
  2. Add confirmation modals to prevent accidentally doing damage.

@klassenjs
Copy link
Member

npm install failed in a massive way on travis. All tests passed locally.

@klassenjs klassenjs merged commit 0684b8b into geomoose:master May 31, 2017
@elil
Copy link
Member

elil commented May 31, 2017

I was thinking simple not fancy. Put the text in the icon/image image from above.

I do find these tool tips quite descriptive and helpful.

@klassenjs
Copy link
Member

That's a bit difficult because the icons aren't images/sprites anymore. They are a single character from Font Awesome rendered by a CSS ::before class over a .

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.

3 participants