Skip to content

Clarity into current editing modes#1147

Merged
BryonLewis merged 7 commits into
mainfrom
fixes-697
Feb 22, 2022
Merged

Clarity into current editing modes#1147
BryonLewis merged 7 commits into
mainfrom
fixes-697

Conversation

@BryonLewis
Copy link
Copy Markdown
Collaborator

@BryonLewis BryonLewis commented Feb 2, 2022

fixes #697
fixes #1172 (Thanks Mary)

Latest update has changed how this works based on mockups.

image
image
image

Switching between different modes (being Creating/Editing OR rectangle/Poly/Line) will display a tooltip for 3 seconds giving information about the mode like "Hit ESC to exist creation", or "select midpoints to add new points to polygon"

These tooltips are still there when returning to hover over the main title section.

Implementation:

  • useModeManager has a canary variable used to determine creating vs editing modes and provides that to the EditorMenu.vue. This is a prop in EditorMenu.vue called editingDetails = 'disabled' | 'Creating' | 'Editing'.
  • There is a computed property called EditorHeader which provides the text and coloring for the buttons based on the editing mode and the editingDetails (I.E When creating a polygon it needs to say creating and the polygon should be colored green).
  • EditorMenu.vue now contains a slot for the deletion controls because they are inside of the Editor gray box in the mockup

@waxlamp
Copy link
Copy Markdown
Member

waxlamp commented Feb 2, 2022

@faiza-a, could you help us out with the styling on this? The intent is to make that red X a dismissal button. My initial thought was to use styling similar to Vuetify dismissible chips, and this mockup isn't quite that. Your expertise would be very useful in guiding us toward the right look.

@subdavis
Copy link
Copy Markdown
Contributor

subdavis commented Feb 2, 2022

Stepping back a second, I want to establish how this works:

  • You use the settings cog next to Tracks (#) to indicate that + Detection should be entered in continuous mode. User intent is to start a round of creating many new boxes in sequence. This is already maybe not the best, but it works. I think a solution to this problem will start with how this part of the UI works, starting with the fact that track and detection are poorly defined and it's not clear how to move between them.
  • The next thing a user will do (after they've figured out that the settings menu changes the type of thing they're creating) is to create the thing using + Track or + Detection. There's a good bit of feedback for this state chage. The cursor gets an icon that follows it around. The editing mode indicator comes on and the active mode lights up. The selected track becomes highlighted.
  • But nothing indicates that we're in continuous mode or how to get out of it.

I think @BryonLewis's proposed solution may conflict with some rules of locality, because you enter a state by pressing a button in one part of the application, but you get out of it by pressing a button in another place. The editing mode radio buttons have the same problem.

I dislike the "Editing Mode" chip because it confuses everyone. I think the information it presents is important, but we should think about where to best represent that information. I also like what you've done with the differentiation between creating and editing.

@waxlamp
Copy link
Copy Markdown
Member

waxlamp commented Feb 2, 2022

Would it be better to place a banner across the annotation widget? Or is that too intrusive/disruptive? Something like "you are now in continuous editing mode".

@faiza-a
Copy link
Copy Markdown

faiza-a commented Feb 2, 2022

@waxlamp I am trying to understand the ask here. having a look at it. will see what solution i can come up with.

Do you think we should meet for this to discuss the concerns/options as group you guys are suggesting above and come to conclusion of how we want to resolve this with minimal changes?

@subdavis subdavis mentioned this pull request Feb 3, 2022
@BryonLewis BryonLewis marked this pull request as ready for review February 11, 2022 15:53
@waxlamp waxlamp requested a review from marySalvi February 15, 2022 16:19
@marySalvi
Copy link
Copy Markdown
Collaborator

marySalvi commented Feb 16, 2022

I think this looks great and is a huge improvement. Just a few thoughts: @BryonLewis @waxlamp @subdavis @faiza-a

  • I think I remember us discussing not using the tooltip and having the help text appear under the Editing Modes text. Is that still preferred?
  • Trying to think about this as a user it seems odd to have 'Delete Polygon' and 'Delete Linestring' but not 'Delete Rectangle'. I understand that deleting the rectangle would mean deleting the track but perhaps it makes sense to allow that here but have the same warning for deleting a track.
  • Along that same train of thought, what do we think of changing the verbiage of 'Editing Rectangle' to 'Editing Bounding Box'? I think this might be clearer for users.
  • Finally I played around with the visibility buttons a bit to try and address the 1px shift:

No Outline
Screenshot from 2022-02-16 16-09-08
Screenshot from 2022-02-16 16-09-24

Static Outline added that is the same color as active button
Screenshot from 2022-02-16 16-14-27

@subdavis
Copy link
Copy Markdown
Contributor

I think I remember us discussing not using the tooltip and having the help text appear under the Editing Modes text. Is that still preferred?

I think so. I haven't gotten around to submitting the change.

Trying to think about this as a user it seems odd to have 'Delete Polygon' and 'Delete Linestring' but not 'Delete Rectangle'. I understand that deleting the rectangle would mean deleting the track but perhaps it makes sense to allow that here but have the same warning for deleting a track.

You're right, it's kinda unusual. It requires the user to understand that all annotations have a rectangle, and deleting is the same as deleting the whole annotation. However, I think most users won't understand that, and will be surprised when deleting a rectangle actually removes all their annotations.

I think the potential to negatively surprise the user is worse than the confusion they might experience from not seeing "delete rectangle", but I could be wrong.

Finally I played around with the visibility buttons a bit to try and address the 1px shift:

Static Outline added that is the same color as active button

I like this option best.

@waxlamp
Copy link
Copy Markdown
Member

waxlamp commented Feb 16, 2022

I think I remember us discussing not using the tooltip and having the help text appear under the Editing Modes text. Is that still preferred?

I think so. I haven't gotten around to submitting the change.

Trying to think about this as a user it seems odd to have 'Delete Polygon' and 'Delete Linestring' but not 'Delete Rectangle'. I understand that deleting the rectangle would mean deleting the track but perhaps it makes sense to allow that here but have the same warning for deleting a track.

You're right, it's kinda unusual. It requires the user to understand that all annotations have a rectangle, and deleting is the same as deleting the whole annotation. However, I think most users won't understand that, and will be surprised when deleting a rectangle actually removes all their annotations.

I think the potential to negatively surprise the user is worse than the confusion they might experience from not seeing "delete rectangle", but I could be wrong.

Obligatory Wikipedia link: https://en.wikipedia.org/wiki/Principle_of_least_astonishment

I think this may be as simple as referring to the rectangle as a track, or otherwise tying the rectangle to the full track. Or perhaps this is just the quantum of training/understanding that users need to have, and there may not be a way to make it simpler via the UI. But I agree that we should not make it easy for the user to accidentally do something they almost certainly don't intend to do (delete a whole track because they think the rectangle is just another form of annotation like polygon or linestring).

Finally I played around with the visibility buttons a bit to try and address the 1px shift:

Static Outline added that is the same color as active button

I like this option best.

Agreed.

@BryonLewis
Copy link
Copy Markdown
Collaborator Author

  • Along that same train of thought, what do we think of changing the verbiage of 'Editing Rectangle' to 'Editing Bounding Box'? I think this might be clearer for users.

That's a good point, I forgot to change the internal names editing/visible modes to user friendly names. LineString doesn't really make sense should it be something like Head/Tail?

@waxlamp
Copy link
Copy Markdown
Member

waxlamp commented Feb 17, 2022

  • Along that same train of thought, what do we think of changing the verbiage of 'Editing Rectangle' to 'Editing Bounding Box'? I think this might be clearer for users.

That's a good point, I forgot to change the internal names editing/visible modes to user friendly names. LineString doesn't really make sense should it be something like Head/Tail?

Keep it as LineString for now. I agree that's not a great name, but "Head/Tail" is too specific alongside other technical terms like "bounding box" and "polygon".

One idea would be to call it a "line segment" since that's what it is in geometric terms. But I don't think we should be making too many ad hoc changes to the language while improving other UI aspects.

@marySalvi
Copy link
Copy Markdown
Collaborator

  • Along that same train of thought, what do we think of changing the verbiage of 'Editing Rectangle' to 'Editing Bounding Box'? I think this might be clearer for users.

That's a good point, I forgot to change the internal names editing/visible modes to user friendly names. LineString doesn't really make sense should it be something like Head/Tail?

Keep it as LineString for now. I agree that's not a great name, but "Head/Tail" is too specific alongside other technical terms like "bounding box" and "polygon".

One idea would be to call it a "line segment" since that's what it is in geometric terms. But I don't think we should be making too many ad hoc changes to the language while improving other UI aspects.

It just occurred to me that making ad hoc changes to language might not be an issue here because I don't think there is currently any user-facing terms for these shapes. Meaning currently it's just the editing buttons but there are no tooltips or descriptions. Am I wrong?

subdavis and others added 2 commits February 18, 2022 13:36
* Switch out tooltip

* Editing bar tweaks

* Remove pre tags

* Visibility spelling
@BryonLewis BryonLewis merged commit 4b6580a into main Feb 22, 2022
@BryonLewis BryonLewis deleted the fixes-697 branch February 22, 2022 15:47
@waxlamp
Copy link
Copy Markdown
Member

waxlamp commented Feb 23, 2022

  • Along that same train of thought, what do we think of changing the verbiage of 'Editing Rectangle' to 'Editing Bounding Box'? I think this might be clearer for users.

That's a good point, I forgot to change the internal names editing/visible modes to user friendly names. LineString doesn't really make sense should it be something like Head/Tail?

Keep it as LineString for now. I agree that's not a great name, but "Head/Tail" is too specific alongside other technical terms like "bounding box" and "polygon".
One idea would be to call it a "line segment" since that's what it is in geometric terms. But I don't think we should be making too many ad hoc changes to the language while improving other UI aspects.

It just occurred to me that making ad hoc changes to language might not be an issue here because I don't think there is currently any user-facing terms for these shapes. Meaning currently it's just the editing buttons but there are no tooltips or descriptions. Am I wrong?

In the delete buttons, the names for the shapes appear. Is that what you mean?

@BryonLewis
Copy link
Copy Markdown
Collaborator Author

In the delete buttons, the names for the shapes appear. Is that what you mean?

The delete button is using also the internal names, like LineString instead of HeadTails. I was specifically talking about the text that is like "Creating LineString" and if that should be "Creating HeadTails" instead.

BryonLewis added a commit that referenced this pull request Feb 24, 2022
* initial test changes

* adding editing Canary for updated info

* Updating styling to match mockup

* minor changes to behavior

* Editing Bar Tweaks (#1175)

* Switch out tooltip

* Editing bar tweaks

* Remove pre tags

* Visibility spelling

* fix 1px shift on visibilty buttons (#1179)

Co-authored-by: Brandon Davis <git@subdavis.com>
Co-authored-by: Mary Salvi <40494088+marySalvi@users.noreply.github.com>
subdavis added a commit that referenced this pull request Mar 11, 2022
* initial test changes

* adding editing Canary for updated info

* Updating styling to match mockup

* minor changes to behavior

* Editing Bar Tweaks (#1175)

* Switch out tooltip

* Editing bar tweaks

* Remove pre tags

* Visibility spelling

* fix 1px shift on visibilty buttons (#1179)

Co-authored-by: Brandon Davis <git@subdavis.com>
Co-authored-by: Mary Salvi <40494088+marySalvi@users.noreply.github.com>
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.

Toolbar Button states cause pixel shifting [FEATURE] Ability to more easily exit from continuous detection creation mode

5 participants