[6290] Move diagram layout configurations from frontend to backend#6298
[6290] Move diagram layout configurations from frontend to backend#6298OrlannC wants to merge 1 commit intoeclipse-sirius:masterfrom
Conversation
frouene
left a comment
There was a problem hiding this comment.
You need to add integration tests for this type of contribution
For inspiration ManageVisibilityActionControllerTests
| import { useLayoutConfigurations } from '../layout/arrange-all/useLayoutConfigurations'; | ||
| import { LayoutConfiguration } from '../layout/arrange-all/useLayoutConfigurations.types'; | ||
| import { ArrangeAllButtonProps, ArrangeAllButtonState } from './ArrangeAllButton.types'; | ||
| import { IconOverlay } from '@eclipse-sirius/sirius-components-core'; |
| Objects.requireNonNull(layoutOptions); | ||
| } | ||
|
|
||
| public static Builder newLayout(String id) { |
There was a problem hiding this comment.
newLayoutConfiguration instead
| DiagramDescription diagramDescription = optionalDiagramDescription.get(); | ||
| var optionalLayoutProvider = this.layoutConfigurationProviders.stream() | ||
| .filter(layoutProvider -> layoutProvider.canHandle(editingContext, diagramContext, diagramDescription)) | ||
| .findFirst(); |
There was a problem hiding this comment.
We shouldn't limit ourselves to the first option. We could consider the layout options offered by several providers
f77a26a to
bb565a7
Compare
| id: ID! | ||
| label: String! | ||
| iconURL: [String!]! | ||
| layoutOptions: String! |
There was a problem hiding this comment.
I've updated the code based on your feedback.
698450e to
ed4ae9e
Compare
|
|
||
| export const useLayoutConfigurations = (): UseLayoutConfigurationsValue => { | ||
| const { diagramDescription } = useDiagramDescription(); | ||
| const { t } = useTranslation('sirius-components-diagrams', { keyPrefix: 'useLayoutConfigurations' }); |
There was a problem hiding this comment.
We used the useTranslation hook only for hard coded static string in the frontend.
Like the label comes from the backend, it should be translated before be sent to the frontend
ed4ae9e to
c55a558
Compare
| */ | ||
|
|
||
| @Component | ||
| public class DefaultLayoutConfiguration implements ILayoutConfigurationProvider { |
There was a problem hiding this comment.
We used to name this kind of class with Provider
Prefer something like DefaultLayoutConfigurationProvider
| return List.of(layoutLayered, layoutRectPacking); | ||
| } | ||
|
|
||
| @SuppressWarnings({"checkstyle:MultipleStringLiterals", "checkstyle:AvoidInlineConditionals"}) |
There was a problem hiding this comment.
I don't see where the AvoidInlineConditionals is necessary
| - https://github.com/eclipse-sirius/sirius-web/issues/6020[#6020] [diagram] Allow the execution of the hardcoded semantic delete tool on a multi selection | ||
| - https://github.com/eclipse-sirius/sirius-web/issues/6246[#6246] [sirius-web] Add missing export of `useCurrentLibrary` and related types. | ||
| - https://github.com/eclipse-sirius/sirius-web/issues/6226[#6226] [diagram] Remove overlap resolution in node layout tools | ||
| - https://github.com/eclipse-sirius/sirius-web/issues/6290[#6290] [diagram] Move diagram layout configurations from frontend to backend |
There was a problem hiding this comment.
Need to be moved in the new 2026.3.0 section
There was a problem hiding this comment.
Sorry, to the 2026.5.0 section
c55a558 to
6e4f240
Compare
| return true; | ||
| } | ||
|
|
||
| @SuppressWarnings("checkstyle:MultipleStringLiterals") |
bff85ca to
778b146
Compare
|
|
||
| LayoutConfiguration layoutLayered = LayoutConfiguration.newLayoutConfiguration("elk-layered") | ||
| .label(this.messageService.diagramLayoutFlow()) | ||
| .iconURL(List.of(DiagramImageConstants.RESET_HANDLES)) |
There was a problem hiding this comment.
Yes, there are no icons for them at the moment, and we can't just take them from the mui library
|
|
||
| LayoutConfiguration layoutRectPacking = LayoutConfiguration.newLayoutConfiguration("elk-rect-packing") | ||
| .label(this.messageService.diagramLayoutCompact()) | ||
| .iconURL(List.of(DiagramImageConstants.RESET_BENDING_POINTS)) |
778b146 to
6582c47
Compare
6582c47 to
b13e767
Compare
bc331da to
534d2e2
Compare
ae91d61 to
8fb205c
Compare
Bug: eclipse-sirius#6290 Signed-off-by: Orlann Cailleau <orlann.cailleau@obeosoft.com>
8fb205c to
5aef59e
Compare

Bug: #6290
Pull request template
General purpose
What is the main goal of this pull request?
Project management
priority:andpr:labels been added to the pull request? (In case of doubt, start with the labelspriority: lowandpr: to review later)area:,difficulty:,type:)CHANGELOG.adocbeen updated to reference the relevant issues?CHANGELOG.adoc? (Including changes in the GraphQL API)CHANGELOG.adoc? For example indoc/screenshots/2022.5.0-my-new-feature.pngArchitectural decision records (ADR)
[doc]?CHANGELOG.adoc?Dependencies
CHANGELOG.adoc?CHANGELOG.adoc?Frontend
This section is not relevant if your contribution does not come with changes to the frontend.
General purpose
Typing
We need to improve the typing of our code, as such, we require every contribution to come with proper TypeScript typing for both changes contributing new files and those modifying existing files.
Please ensure that the following statements are true for each file created or modified (this may require you to improve code outside of your contribution).
useMutation<DATA_TYPE, VARIABLE_TYPE>(…)useQuery<DATA_TYPE, VARIABLE_TYPE>(…)useSubscription<DATA_TYPE, VARIABLE_TYPE>(…)useMachine<CONTEXT_TYPE, EVENTS_TYPE>(…)useState<STATE_TYPE>(…)?.(if the GraphQL API specifies that a field cannot benull, do not treat it has potentiallynullfor example)let diagram: Diagram | null = null;)Backend
This section is not relevant if your contribution does not come with changes to the backend.
General purpose
Architecture
Review
How to test this PR?