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

Compatibility layer: handle DotDescription style for Bordered Nodes #897

Closed
1 task done
AxelRICHARD opened this issue Dec 17, 2021 · 5 comments · Fixed by #899
Closed
1 task done

Compatibility layer: handle DotDescription style for Bordered Nodes #897

AxelRICHARD opened this issue Dec 17, 2021 · 5 comments · Fixed by #899

Comments

@AxelRICHARD
Copy link
Contributor

  • I have checked that this feature has not yet been suggested by someone else.

Expected behavior

Handle Dot style for Bordered Nodes in the compatibility layer.
A Dot is simply a RectangularNodeStyle with a radius = 100.

Describe alternatives you've considered

A new CircleNodeStyle could be implemented instead of using RectangularNodeStyle with a radius = 100

@sbegaudeau sbegaudeau added this to the 2022.01.0 milestone Dec 17, 2021
AxelRICHARD added a commit to AxelRICHARD/sirius-components that referenced this issue Dec 17, 2021
Bug: eclipse-sirius#897
Change-Id: Id41b42e980bd80b67778a9195dd51013636edc1d
Signed-off-by: Axel RICHARD <axel.richard@obeo.fr>
@sbegaudeau sbegaudeau linked a pull request Dec 17, 2021 that will close this issue
1 task
sbegaudeau pushed a commit to AxelRICHARD/sirius-components that referenced this issue Dec 20, 2021
Bug: eclipse-sirius#897
Signed-off-by: Axel RICHARD <axel.richard@obeo.fr>
sbegaudeau pushed a commit that referenced this issue Dec 20, 2021
Bug: #897
Signed-off-by: Axel RICHARD <axel.richard@obeo.fr>
@pcdavid
Copy link
Member

pcdavid commented Dec 27, 2021

I don't understand how we can says that this implements the DotDescription from Sirius Desktop, which are represented using a circle. The changelog should a least mention what "handled" means here.

@sbegaudeau
Copy link
Member

A SVG rectangle with a 100 radius is a circle. No change require on the frontend this way.

@pcdavid
Copy link
Member

pcdavid commented Dec 30, 2021

But the border radius is an absolute dimension, not a percentage. It will only render as a Sirius Desktop "Dot" at small sizes:

2021-12-30_10-22

Yes, in SVG the rx attribute can be a percentage, but in our case RectangularNodeStyle.getBorderRadius() is an int which we always convert as a plain length.

I guess we could embrace the fact that we will render as SVGs, make RectangularNodeStyle.getBorderRadius() a string and return "100%" in this case. Which would offer more flexibility to programatically defined modelers to chose between absolute lengths and percentage. Or add a "hack" to interpret negative values as percentage in RectangleView.tsx and return -100 here.

@sbegaudeau
Copy link
Member

You're right, I believed that we were interpreting it as a percentage on the frontend. We could indeed make RectangularNodeStyle.getBorderRadius() a string. We could even create different border radius configurations for each corner. Using that philosophy, I'm not sure why we would not remove both RectangularNodeStyle.getBorderSize() and RectangularNodeStyle.getBorderRadius() in favor of a string based RectangularNodeStyle.getBorder() since it would allow us to create way more complex borders.

@pcdavid
Copy link
Member

pcdavid commented Jan 3, 2022

👍 to "embrace SVG and CSS" (and not try to hide/abstract their more powerful features)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants