Improve handling of change bounds in several areas #344
Conversation
There was a problem hiding this comment.
Thanks a lot Martin.
I really like the centralized approach of handling bound changes.
I have retested everything and can confirm that all mentioned bugs are fixed and (almost) everything works as expected.
There seems to be a bug when resizing elements close to the diagram border when projetion bars are visible:
resize-bug.mp4
On master the resize works as expected in this case.
Other than that i also noticed that the a11y keyboard resize tool currently does not respect grid size i.e. resizing with +/- does not snap the size to the grid. But we can handle that in a follow-up PR if necessary.
- Introduce change bounds manager to centralize bounds-related services -- Bounds changes through position snapping and movement restriction -- Validation for size and position of an element -- Customizable methods for when to use move and resize options - Introduce change bounds tracker for moves and resizes -- Tracker calculates move on diagram and calculates move and resizes -- Tracker supports options on which parts of the process are applied - Provide moveable wrappers for resize and routing handles Fixes eclipse-glsp/glsp#1337 - Extend current resize capabilities -- Introduce mode for symmetric resize -- Introduce one-dimensional resize on top, right, bottom and left side Fixes eclipse-glsp/glsp#1338 Fixes eclipse-glsp/glsp#1339 - Fix elements moving during resizing when hitting minimum bounds -- Store calculated minimum size from layouter in element -- Adapt resize so we do not produce invalid sized bounds Fixes eclipse-glsp/glsp#1340 Minor: - Ensure we get proper cursor feedback when hovering over resize handle - Add additional convenience functions - Add origin viewport command for convenience Contributed on behalf of Axon Ivy AG
- Fix issue with wrong coordinates on project bars during move/resize - Fix move during resize for constrained, symmetric resize - Ensure symmetric resize does not trigger deselect of element - Remove !important from CSS rules - Allow undefined unsnap modifier for adopters - Use isSizeable for debug decoration to avoid wrong edge decoration - Remove non-generic getAbsoluteEdgeBounds - Better unify the helper functions for CSS classes - Fix deprecation warnings - Minor cleanup Contributed on behalf of Axon Ivy AG
martin-fleck-at
force-pushed
the
issues/1337-1340
branch
from
9229bcb
to
7c15c61
Compare
|
@tortmayr I think I managed to address all the issues, I'd really appreciate it if you could have another, more detailed look at this. Thank you very much! |
There was a problem hiding this comment.
Thanks Martin. All issues have been addressed and everything seems to work as expected now. Nice job!
I encountered one issue with the recently introduced debug module. It also decorates validation markers (and potentially other decorations) which looks a bit confusing. We should probably not decorate decorations at all:
packages/client/css/helper-lines.css
Outdated
| @@ -16,8 +16,8 @@ | |||
|
|
|||
| .helper-line { | |||
| pointer-events: none; | |||
| stroke: red; | |||
| stroke-width: 1px; | |||
| stroke: #f7086c; | |||
There was a problem hiding this comment.
Should we maybe use a less aggressive color than red for the helper lines?
There was a problem hiding this comment.
Do you maybe have a suggestion? I tried to choose a softer red, more pink-ish color to make it less look like an error. However, I couldn't quickly find a color that has a good contrast to most other colors and still look very visible on the blue-ish background.
There was a problem hiding this comment.
I pushed an update where I set the stroke width back to 1 and use the blue that we also use for the resize handles, please have a look at that.
| @@ -49,7 +49,7 @@ declare module 'sprotty-protocol/lib/utils/geometry' { | |||
| * @param right right bounds | |||
| * @returns true if the two bounds are equal | |||
| */ | |||
| function equals(left: Bounds, right: Bounds): boolean; | |||
| function equals(left: Bounds, right: Bounds, eps?: number): boolean; | |||
There was a problem hiding this comment.
Missing @param for the eps param
|
@tortmayr Thank you, I pushed an update that should address the remaining issues. Now we just need to agree on a color ;-) |
What it does
Improve handling of change bounds
Introduce change bounds manager to centralize bounds-related services
-- Bounds changes through position snapping and movement restriction
-- Validation for size and position of an element
-- Customizable methods for when to use move and resize options
Introduce change bounds tracker for moves and resizes
-- Tracker calculates move on diagram and calculates move and resizes
-- Tracker supports options on which parts of the process are applied
Provide moveable wrappers for resize and routing handles
Fixes eclipse-glsp/glsp#1337
-- Introduce mode for symmetric resize
-- Introduce one-dimensional resize on top, right, bottom and left side
Fixes eclipse-glsp/glsp#1338
Fixes eclipse-glsp/glsp#1339
-- Store calculated minimum size from layouter in element
-- Adapt resize so we do not produce invalid sized bounds
Fixes eclipse-glsp/glsp#1340
Minor:
Please note this PR builds upon #343 and should only be merged afterwards!
How to test
Follow-ups
Changelog
As soon as we agree on the PR, I'll write a paragraph about the changes ;-)