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

feat: clearly distinguish select and hover states #640

Merged
merged 10 commits into from
May 23, 2022
Merged

Conversation

nikku
Copy link
Member

@nikku nikku commented May 18, 2022

This proposes a rework of our hover and select outline, as well as interaction (resize, drag, ...) handles:

  • No hover state (no blinking as you move the mouse)
  • Clear, solid outline for selected elements
  • Clear, no opacity, unified size handles to interact with diagram elements (resize, bendpoints, ...)

Role model for this change is Google Slides, but similar approaches can be found in many different apps.


capture WUR6jb_optimized


Try it out locally:

npx @bpmn-io/sr bpmn-io/bpmn-js#develop -l bpmn-io/diagram-js#select-hover-states

TODO


Related to bpmn-io/bpmn-js#1616

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label May 18, 2022
@nikku nikku marked this pull request as ready for review May 18, 2022 10:05
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels May 18, 2022
@philippfromme
Copy link
Contributor

philippfromme commented May 18, 2022

In my opinion, we should not only increase the line width but also the size of the handles (resize, segment move), which are smaller than bend point move handles. I proposed this years ago.

brave_oFzsDMFUMP

@andreasgeier
Copy link

andreasgeier commented May 18, 2022

I'd go with 2px. Consider, that single select is the default case and it should help to better see/find the selected element. Particularly in case of clicking on an error message references.

Please avoid a hover state on selected elements. It's not common and I believe it is confusing.

edit: no hover on selected elements seems already work as expected

@nikku
Copy link
Member Author

nikku commented May 18, 2022

@philippfromme You have your adjustments on a branch?

@philippfromme
Copy link
Contributor

@christian-konrad
Copy link

2px looks better. 1px makes sense for high-precision drawing tools (adobe, affinity, sketch...), but for a rapid modeling tool, quick distinguishability and clickability is more important.

assets/diagram-js.css Outdated Show resolved Hide resolved
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels May 18, 2022
@nikku
Copy link
Member Author

nikku commented May 18, 2022

I'll provide another version that incorporates @philippfromme styles. If we go for #640 (comment), more general clarity on our UI controls helps.

@nikku
Copy link
Member Author

nikku commented May 18, 2022

2px looks better. 1px makes sense for high-precision drawing tools (adobe, affinity, sketch...), but for a rapid modeling tool, quick distinguishability and clickability is more important.

Visual sizing has no impact for clickability. This is really about what we want to be, a happy looking sketching tool, or a slight less in your face app. Miro for example goes for 1px, no problem and looks clean because of that.

@nikku
Copy link
Member Author

nikku commented May 19, 2022

Accounting for @philippfromme feedback but also taking into consideration other drawing apps I've reworked this PR:

  • Outline is solid, if it is shown
  • Hover outline is gone (no-one has it)
  • Handles are not transparent, and sized appropriately
  • No two pixel border, as this looks toyish (and other tools don't have it, too)

capture vdZvZR_optimized


I've considered the following tools for comparison:

  • Google Slides
  • Excalidraw
  • Miro

@currandwyer
Copy link

I think this is looking great! Cool that we're working on it. One question for you @nikku - why don't we ease the corners of the selection boxes? I think this looks much more refined, given that almost everything being selected also has rounded corners. All we would need to do is decide on a different behavior for pools, which seems doable to me.

@philippfromme
Copy link
Contributor

philippfromme commented May 19, 2022

why don't we ease the corners of the selection boxes?

@currandwyer Do you mean border radius?

We could use the same border radius as we do for forms.

image

@nikku
Copy link
Member Author

nikku commented May 19, 2022

@currandwyer Thanks for your feedback!

As mentioned here I'd love to take easing (rounding) borders out of this topic, as "doing exceptions based on element types" is not a direction I'd like to go right now. It is not only about pools, but about gateways, groups, DMN decisions, ...

Selection outline shall be a general concern and not depend on the shape of the element; at least this is something that I'd not expect as a user. If you consider Miro for example, they agree with me:

image

@nikku
Copy link
Member Author

nikku commented May 19, 2022

@philippfromme That is a minimal one, right? I'll give it a shot and report back how that looks like.

@currandwyer
Copy link

currandwyer commented May 19, 2022

Yeah @philippfromme I think even just having 1-2 px corner radius is nice - it's subtle, but it softens the look a bit, and shows that you thought about it. By the way, in Operate we already have a relatively large corner radius:
Screen Shot 2022-05-19 at 10 55 55

@nikku
Copy link
Member Author

nikku commented May 19, 2022

Playing around with radiuses:

As close to Task as possible (10px)

capture uJCxNS_optimized

Minimal (3px)

capture 6lFYks_optimized

What do you think?

@nikku
Copy link
Member Author

nikku commented May 19, 2022

@currandwyer Thanks for the operate screen. Does the user interact with Participants there? How does operate handle selection on DMN DRD diagrams (with decisions and text annotations)?

image

@christian-konrad
Copy link

christian-konrad commented May 19, 2022

@nikku is it possible (and to what cost) to have the outline thickness be fixed, so not to scale with the canvas when you zoom?

@christian-konrad
Copy link

@nikku interesting thought to get rid of the hover outline at all, this solves many problems with multi-select.
But, would it be possible then to turn the cursor into a drag cursor (like in Google Slides / Excalidraw)
(Just a thought, I am not even sure if I like it)

@nikku
Copy link
Member Author

nikku commented May 19, 2022

@nikku is it possible (and to what cost) to have the outline thickness be fixed, so not to scale with the canvas when you zoom?

Not easily possible.

But, would it be possible then to turn the cursor into a drag cursor.

When exactly? We could do it. But I don't miss it (and no-one ever did).
It would also cause conflicts, i.e. in places where you could both start dragging, but also direct editing via double click. TLDR: Let's not optimize it in this PR 😉

@nikku
Copy link
Member Author

nikku commented May 19, 2022

I was wondering, shouldn't the outline always be on top? thinking Like, not covered by the boundary event.

Let us follow up with this in a new issue @philippfromme.

I've checked other tools and that is exactly how they do it. Our multi-select outline also works like this:

image

@philippfromme
Copy link
Contributor

Let us follow up with this in a new issue @philippfromme.

👍🏻

@nikku nikku requested a review from smbea May 20, 2022 07:55
@christian-konrad
Copy link

Just found this bug while testing, which could be caused by this bpmn-io/bpmn-js#1657

@christian-konrad
Copy link

I am not 100% happy with having no hover state anymore, but as this solution solves current problems, please move forward with it.
Reconsider hovering if a designer comes up with it and good reasoning.
(There isn't even a real best practice there, there are vector, drawing, model etc tools with and without hover states)

@nikku nikku mentioned this pull request May 20, 2022
@nikku
Copy link
Member Author

nikku commented May 20, 2022

Potential follow up once we got this merged (#643), incorporating @christian-konrad earlier comment (#640 (comment)).

I did check other tools and a majority do it as suggested.

nikku and others added 6 commits May 20, 2022 14:10
* remove hover outline (not present in comparable editor apps)
* make select outline solid so it is easier to recognize
* make drag handles solid so they are easier to recognize
* unify drag handle sizes
* separate primary from secondary selection
* hide bendpoint + segment draggers on multi-select
Use same color like selection outline to match users mental model.
@nikku
Copy link
Member Author

nikku commented May 23, 2022

@philippfromme As discussed, please leave this leave your review.

@philippfromme
Copy link
Contributor

As discussed, please leave this.

I was about to approve. What do you mean?

@nikku
Copy link
Member Author

nikku commented May 23, 2022

Hahaha. leave you review :).

image

Copy link
Contributor

@philippfromme philippfromme left a comment

Choose a reason for hiding this comment

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

This is such a neat package of improvements! 🏅 I love it!

@nikku nikku merged commit 95312b4 into develop May 23, 2022
@nikku nikku deleted the select-hover-states branch May 23, 2022 13:12
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants