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

Creation tool uses the wrong location when creating a border node from inner container in a Stack #396

Open
mypsycho opened this issue May 22, 2024 · 4 comments · May be fixed by #455
Open
Assignees

Comments

@mypsycho
Copy link

Use case:
It is possible in ContainerMapping to create an inner ContainerMapping using childrenPresentation as ContainerLayout.VERTICAL_STACK.
This designs creates a separation between the title zone and the Free form zone.
In this design, the click zone to create a border node is very limited.
A solution is to add the inner ContainerMapping in extraMappings of the NodeCreation tool.

The issue is, in this configuration, location to create the view of a new element is wrong :
image

@mypsycho
Copy link
Author

Analysis:
The issue takes place in the following method:

private LayoutDataResult updateAbstractDNode_ownedBorderedNodes_Bounds(View createdView) {

On classic mapping (ContainerMapping with FreeForm),
this method can find the proper layoutData as the layoutData target the container of the created portNode.

But when using a combinaison ContainerMapping{vertical_stack} + ContainerMapping{FreeForm}, layoutData is not found and port is placed on default location (left/top corner).

A solution may be to update the behavior of SiriusLayoutDataManager.INSTANCE.getData(portNode, true) invocation to also consider that layoutData targeting a child of the container is valid match.

See this method to search the proper location:

@lredor lredor self-assigned this May 23, 2024
@lredor
Copy link
Contributor

lredor commented May 23, 2024

Step to reproduce:

  • Import project Issue396 from Issue396ProjectSample.zip
  • Open the diagram root class diagram
  • Execute the tool EReference by clicking in the middle of the right border of Class1
  • Expected: A border node is created on the click location.
  • Observed: A border node is created in the top left corner of Class1.
    2024-05-23_17h03_34

lredor added a commit that referenced this issue May 24, 2024
DRAFT: No fully sure of the impact of added test in the subclasses. Must
be verify after merged.

Bug: #396
@lredor lredor linked a pull request May 24, 2024 that will close this issue
@lredor
Copy link
Contributor

lredor commented May 28, 2024

Step to reproduce:

Issue396ProjectSample-v2.zip
This project sample is more complete:

  • a standard container (free form),
  • a list container,
  • a container with several levels of regions.
    With a tool associated with each type of container, it allows you to compare the different cases.

@lredor
Copy link
Contributor

lredor commented May 28, 2024

@mypsycho Thank for your analysis, however I think it will be simpler to do the right processing upstream. That is to say, add the correct "LayoutData", with the correct parent edit part associated with the tool, when creating commands in org.eclipse.sirius.diagram.ui.graphical.edit.policies.NodeCreationEditPolicy.
Indeed, doing this in updateAbstractDNode_ownedBorderedNodes_Bounds would force us to redo the calculation of "relative/absolute" positions when we do not have all the necessary elements.

lredor added a commit that referenced this issue Sep 12, 2024
- Constructors of CreationUtil have been improved to directly have
LayoutData as parameter instead of location and size. Indeed, the
location is relative to the parent edit part and for border nodes, the
parent edit part is not systematically the "host" of the called policy.
- In NodeCreationEditPolicy, in case of border nodes, the "host edit
part" is not systematically considered (to compute the RootLayoutData
for example). The correct edit part, a parent of the host, is computed
according to the mapping defined in the used tool.

Bug: #396
@lredor lredor linked a pull request Sep 12, 2024 that will close this issue
lredor added a commit that referenced this issue Sep 12, 2024
- Constructors of CreationUtil have been improved to directly have
LayoutData as parameter instead of location and size. Indeed, the
location is relative to the parent edit part and for border nodes, the
parent edit part is not systematically the "host" of the called policy.
- In NodeCreationEditPolicy, in case of border nodes, the "host edit
part" is not systematically considered (to compute the RootLayoutData
for example). The correct edit part, a parent of the host, is computed
according to the mapping defined in the used tool.

Bug: #396
lredor added a commit that referenced this issue Sep 13, 2024
- Constructors of CreationUtil have been improved to directly have
LayoutData as parameter instead of location and size. Indeed, the
location is relative to the parent edit part and for border nodes, the
parent edit part is not systematically the "host" of the called policy.
- In NodeCreationEditPolicy, in case of border nodes, the "host edit
part" is not systematically considered (to compute the RootLayoutData
for example). The correct edit part, a parent of the host, is computed
according to the mapping defined in the used tool.

Bug: #396
lredor added a commit that referenced this issue Sep 17, 2024
- Constructors of CreationUtil have been improved to directly have
LayoutData as parameter instead of location and size. Indeed, the
location is relative to the parent edit part and for border nodes, the
parent edit part is not systematically the "host" of the called policy.
- In NodeCreationEditPolicy, in case of border nodes, the "host edit
part" is not systematically considered (to compute the RootLayoutData
for example). The correct edit part, a parent of the host, is computed
according to the mapping defined in the used tool.

Bug: #396
lredor added a commit that referenced this issue Sep 27, 2024
- Constructors of CreationUtil have been improved to directly have
LayoutData as parameter instead of location and size. Indeed, the
location is relative to the parent edit part and for border nodes, the
parent edit part is not systematically the "host" of the called policy.
- In NodeCreationEditPolicy, in case of border nodes, the "host edit
part" is not systematically considered (to compute the RootLayoutData
for example). The correct edit part, a parent of the host, is computed
according to the mapping defined in the used tool.

Bug: #396
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 a pull request may close this issue.

2 participants