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

Allow to add a vertex to a collision mask by pulling an edge. #4778

Merged
merged 13 commits into from Jan 5, 2023

Conversation

D8H
Copy link
Collaborator

@D8H D8H commented Jan 3, 2023

@D8H D8H changed the title Allow to add a vertex to a collision mask by pulling an edge. WIP: Allow to add a vertex to a collision mask by pulling an edge. Jan 3, 2023
@D8H D8H changed the title WIP: Allow to add a vertex to a collision mask by pulling an edge. Allow to add a vertex to a collision mask by pulling an edge. Jan 4, 2023
@LuniMoon
Copy link
Collaborator

LuniMoon commented Jan 5, 2023

UX frictions identified during individual testing to consider:

On Mobile:
Video with the interaction: https://user-images.githubusercontent.com/103995399/210753877-5a5ed200-4640-4e57-bd46-d19bd7092a4d.mp4

  1. Zooming in/out with the "pinch" interaction (my first reaction since this is common mobile interaction, instead of using the zoom bar) makes a mess with vectors. I would either a "AAmobileU I can zoom in/out pinching the collision mask workspace" or if that's too complex, "block" the possibility of adding a new vector when the user "pinches" the screen (probably hard too?)
  2. Moving around the workspace (during zoom-in) to edit other vectors (with one hold/drag interaction of my finger) also makes a vector mess. This interaction is tricky cause we need to distinguish between wanting to move a vector or navigate the space
  3. Better visual feedback for "press and hold" vectors: the finger "hides" the vector just under, so there is no way to tell if it is selected or placed on the proper location. Mobile devices (a representation of Apple's on this case) had some "magnifying glass" UI that let the user know what they were selecting.

Screenshot 2023-01-05 at 10 44 42

Question: do we still need the drag for XY lines? It was there to re-order vector position, but I'm not entirely sure that they should be there anymore (in order to prioritise a visual, over a numeric Interface).

Uncertainties to observe during tests (on recruiting status):

  • Are vector colors visible enough?
  • How do users click to add a vector on the workspace (click alone or click and drag?)
  • How do users feel about having "vectors disappearing" when they're merged into a straight line?
  • Would having a change on cursor (move, add, subtract) would clarify these interactions?

@D8H D8H marked this pull request as ready for review January 5, 2023 10:35
@D8H D8H requested a review from 4ian as a code owner January 5, 2023 10:35
Copy link
Collaborator

@ClementPasteau ClementPasteau left a comment

Choose a reason for hiding this comment

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

lgtm, just a few comments

/**
* @returns true if the vertex should be deleted.
*/
const magnetDraggedVertexForDeletion = (): boolean => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this function should have a clearer name to say what it does.
But it's confusing because it does both "update the draggedVertex" and "say if it should be deleted"
maybe call it updateMagnetDraggedVertexForDeletion()

Copy link
Collaborator Author

@D8H D8H Jan 5, 2023

Choose a reason for hiding this comment

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

I though is was a verb, I'll replace it with "magnetize".

@@ -127,32 +270,45 @@ const CollisionMasksPreview = (props: Props) => {
/>
);
})}
{mapVector(polygons, (polygon, i) => {
{mapVector(polygons, (polygon, polygonIndex) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

way clearer thanks

@@ -43,7 +43,7 @@ const horizontalMosaicNodes: EditorMosaicNode = {
direction: 'row',
first: 'preview',
second: 'properties',
splitPercentage: 50,
splitPercentage: 66.67,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this look fine on all resolutions? (+mobile?)

Copy link
Collaborator Author

@D8H D8H Jan 5, 2023

Choose a reason for hiding this comment

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

On mobile, it uses the vertical layout. Even with a 1,000 pixels wide window, it seems ok. It allows a bigger image.

@D8H D8H merged commit c0ba26f into master Jan 5, 2023
@D8H D8H deleted the pull-new-vertex branch January 5, 2023 11:58
@LuniMoon
Copy link
Collaborator

LuniMoon commented Jan 9, 2023

Vector Wokspace - User testing 7/01/2023 (Thanks @roshh and @beanmatt for your valuable input!):

  • 2 out of 2 users really liked the new vector workspace
  • 1 out of 2 users figured quickly that pressing and dragging, added a new vector
  • 1 out of 2 users clicked once -without dragging- to add a vector. The system didn't add the vector.
    They later figure out that clicking while dragging would do what they wanted it to do.
  • 2 out of 2 users tried to delete a vector on the workspace by selecting the vector, and pressing "⌦" (erase/delete) on the keyboard
  • 2 out of 2 users understood that pressing the trashcan deleted a selected vector on the workspace
  • 2 out of 2 users didn't figure out how to delete a vector (both pressed "⌦" to do so)
  • 1 out of 2 users took time to find out that placing a vector on a straight line on the workspace, deletes the vector

Other findings:

  • Vector color doesn't seem to be an issue (black, red, blue)
  • Changing the cursor status might help the user understand when a vector is being deleted on a straight line
  • To complete the (un-existent implementation yet) "move collision mask to the right", users:
    • Tried to shift select multiple coordinates on the XY list to later move it
    • Double clicked on the center of the mask

To do:

  • Allow users to delete a vector by pressing "⌦" on their keyboard
  • When a vector is been dragged to a straight line (to be deleted), change the cursor indicator to show a minus symbol to tell the user their vector will be deleted (see image)

Screenshot 2023-01-09 at 14 13 51

Nice to haves:

  • Allow users to move selected vectors with keyboard arrows ← → ↑ ↓
  • Allow keyboard input to undo and redo actions
  • Allow users to double click on collision masks (to select all vectors) and move them with mouse or keyboard arrows (for more precision)

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

3 participants