[3429] Add diagram filter view in diagram panel #3430

gdaniel · 2024-04-29T13:54:28Z

Bug: #3429

Pull request template

General purpose

What is the main goal of this pull request?

Project management

Has the pull request been added to the relevant project and milestone? (Only if you know that your work is part of a specific iteration such as the current one)
Have the priority: and pr: labels been added to the pull request? (In case of doubt, start with the labels priority: low and pr: to review later)
Have the relevant issues been added to the pull request?
Have the relevant labels been added to the issues? (area:, difficulty:, type:)
Have the relevant issues been added to the same project and milestone as the pull request?
Has the CHANGELOG.adoc been updated to reference the relevant issues?
Have the relevant API breaks been described in the CHANGELOG.adoc? (Including changes in the GraphQL API)
In case of a change with a visual impact, are there any screenshots in the CHANGELOG.adoc? For example in doc/screenshots/2022.5.0-my-new-feature.png

Architectural decision records (ADR)

Does the title of the commit contributing the ADR start with [doc]?
Are the ADRs mentioned in the relevant section of the CHANGELOG.adoc?

Dependencies

Are the new / upgraded dependencies mentioned in the relevant section of the CHANGELOG.adoc?
Are the new dependencies justified in the CHANGELOG.adoc?

Frontend

This section is not relevant if your contribution does not come with changes to the frontend.

General purpose

Is the code properly tested? (Plain old JavaScript tests for business code and tests based on React Testing Library for the components)

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).

Backend

This section is not relevant if your contribution does not come with changes to the backend.

General purpose

Are all the event handlers tested?
Are the event processor tested?
Is the business code (services) tested?
Are diagram layout changes tested?

Architecture

Are data structure classes properly separated from behavioral classes?
Are all the relevant fields final?
Is any data structure mutable? If so, please write a comment indicating why
Are behavioral classes either stateless or side effect free?

Review

How to test this PR?

Start a Sirius Web application (new architecture)
Create a studio
Open the diagram representation inside DomainNewModel
Click on the diagram filters tool in the diagram panel
Select some nodes and apply tools on them
The diagram should be updated to reflect the tools you applied

Has the Kiwi TCMS test suite been updated with tests for this contribution?

.../org/eclipse/sirius/web/application/views/explorer/services/ExplorerDescriptionProvider.java

packages/diagrams/frontend/sirius-components-diagrams/src/renderer/panel/DiagramPanel.tsx

...diagrams/frontend/sirius-components-diagrams/src/renderer/panel/DiagramPanelContribution.tsx

...irius/web/application/views/representations/services/DiagramFilterRefreshPolicyProvider.java

.../org/eclipse/sirius/web/application/views/explorer/services/ExplorerDescriptionProvider.java

...lication/views/diagramfilter/services/api/IDiagramFilterSplitButtonContributionProvider.java

.../org/eclipse/sirius/components/forms/tests/graphql/DiagramFilterEventSubscriptionRunner.java

...ava/org/eclipse/sirius/web/application/views/diagramfilter/services/DiagramFilterHelper.java

...n/java/org/eclipse/sirius/components/collaborative/forms/api/DiagramFilterConfiguration.java

sbegaudeau

I have not executed the feature yet to play with it but the code seems nice. Given the complexity of the work that you've done, don't be surprised by the number of comments. It was expected.

sbegaudeau · 2024-05-07T07:32:41Z

CHANGELOG.adoc

@@ -109,6 +109,11 @@ image:doc/screenshots/inside_outside_labels.png[Isinde outside label example, 70
 - https://github.com/eclipse-sirius/sirius-web/issues/3382[#3382] [diagram] Add a tool section in Node and Edge palettes with tools to hide/show/reset elements and their children.
 - https://github.com/eclipse-sirius/sirius-web/issues/3135[#3135] [diagram] Add new overflow strategies for node label : _wrap_ and _ellipsis_
 - https://github.com/eclipse-sirius/sirius-web/issues/3425[#3425] [diagram] Make the direction applied during _arrangeAll_ configurable.
+- https://github.com/eclipse-sirius/sirius-web/issues/3429[#3429] [diagram] Add a diagram filter view in the diagram panel.
+This view presents diagram elements in a tree and allows to select them and apply tools on the selection (e.g. hide, show, collapse).


apply tools on the selection

I would rather say "update their visibility and status" or something like that because "tools" are defined by the specifier with any behavior they want. With this sentence, consumers of Sirius Web may believe that we have added the ability to execute tools on a batch of elements at once. It could be a nice addition in the future but it's not what you have done here.

sbegaudeau · 2024-05-07T07:34:57Z

packages/diagrams/frontend/sirius-components-diagrams/src/renderer/panel/DiagramPanel.tsx

@@ -57,6 +60,7 @@ export const DiagramPanel = memo(
    });

    const { readOnly } = useContext<DiagramContextValue>(DiagramContext);
+    const { views } = useContext<DiagramPanelViewContextValue>(DiagramPanelViewContext);


Leverage the existing extension capabilities to add this new mechanism. See useComponents for example. You should end up with something like this:

const diagramPanelActionComponents: ComponentExtension<DiagramPanelActionProps>[] = useComponents(diagramPanelActionExtensionPoint);

sbegaudeau · 2024-05-07T07:41:43Z

packages/diagrams/frontend/sirius-components-diagrams/src/renderer/panel/DiagramPanel.tsx

+            {views.map((view) => {
+              return (
+                <DiagramPanelContribution
+                  view={view}
+                  editingContextId={editingContextId}
+                  diagramId={diagramId}
+                  key={view.title}
+                />
+              );
+            })}


Just let people contribute any component they want here but ask for one which accept specific props. Something like:

interface DiagramPanelActionProps { editingContextId: string; diagramId: diagramId; }

You can thus remove DiagramPanelContribution.tsx and DiagramPanelContribution.types.ts from here (the code would be used specifically in your filter action) but not everybody wants that specific behavior

sbegaudeau · 2024-05-07T07:43:55Z

packages/diagrams/frontend/sirius-components-diagrams/src/contexts/DiagramPanelViewContext.ts

+ * Contributors:
+ *     Obeo - initial API and implementation
+ *******************************************************************************/
+import React from 'react';


View has multiple very specific meaning in Sirius Web but it's mostly the root concept where representation description are created or the workbench views like the explorer or the details/properties view. Try not to use this term as much as possible, there's not reason to have anything in this contribution named view.

You don't need a new context for this feature, use the extension capabilities of sirius-components-core. You can remove this file entirely

Renamed the contribution to "DiagramFilter", and the component containing the form to "DiagramFilterForm"

sbegaudeau · 2024-05-07T07:45:57Z

...s/diagrams/frontend/sirius-components-diagrams/src/contexts/DiagramPanelViewContext.types.ts

+ * Contributors:
+ *     Obeo - initial API and implementation
+ *******************************************************************************/
+import { WorkbenchViewComponentProps } from '@eclipse-sirius/sirius-components-core';


Can be removed in favor of the extension capabilities of sirius-components-core. Do not use WorkbenchViewComponentProps, it exists to contribute new panels on the side of the workbench like the explorer. You can remove this file entirely

sbegaudeau · 2024-05-07T08:42:53Z

...ava/org/eclipse/sirius/web/application/views/diagramfilter/services/DiagramFilterHelper.java

+    }
+
+    public IStatus sendDiagramEvent(VariableManager variableManager, IDiagramInput diagramInput) {
+        IRepresentationEventProcessor representationEventProcessor = variableManager.get("diagramEventProcessor", IRepresentationEventProcessor.class).get();


Use some constants to share the name of the variable between this class and DiagramFilterDescriptionProvider

sbegaudeau · 2024-05-07T08:43:09Z

...ava/org/eclipse/sirius/web/application/views/diagramfilter/services/DiagramFilterHelper.java

+    private final Logger logger = LoggerFactory.getLogger(DiagramFilterHelper.class);
+
+    public Set<String> getSelectedElementIds(VariableManager variableManager) {
+        Map<String, Boolean> checkMap = variableManager.get(DiagramFilterDescriptionProvider.SELECTED_TREE_NODES, Map.class).get();


Don't use .get() on optional without testing that the value is present first.

sbegaudeau · 2024-05-07T08:45:17Z

...rc/main/java/org/eclipse/sirius/web/services/diagramfilter/PinButtonDescriptionProvider.java

+                        .foregroundColor("#000000")
+                        .build()
+                )
+                .helpTextProvider(variableManager -> "Pin selected elements")


Use the internationalization support for text sent back to the end user (this applies to other button description provider)

sbegaudeau · 2024-05-07T08:46:28Z

...va/org/eclipse/sirius/web/application/controllers/diagrams/DiagramFilterControllerTests.java

+                    var optTreeWidget = group.getWidgets().stream()
+                        .filter(TreeWidget.class::isInstance)
+                        .map(TreeWidget.class::cast)
+                        .findFirst();


=you can continue using the form navigator to retrieve the tree widget and ensure that it is present

I should know that 🙂
Done

sbegaudeau · 2024-05-07T08:48:53Z

...e/sirius/web/application/views/diagramfilter/services/BrightenButtonDescriptionProvider.java

+                        .backgroundColor("#ffffff")
+                        .foregroundColor("#000000")


Do you need to change the default style of the buttons? Because #000000 is not the color of the text in Sirius Web and in most applications based on Sirius Web.

@mbats asked me to put the background in white, because the default colors for the buttons weren't working fine (see screenshot below).

The default foreground color for buttons is white, so I had to change it to make the text visible. I updated it to use #261E58 to match the default text color in Sirius Web, but it will be hardcoded and won't change if the theme changes. Let me know if this is fine for you or if we need something else

Bug: eclipse-sirius#3429 Signed-off-by: Gwendal Daniel <gwendal.daniel@obeosoft.com>

gdaniel requested review from pcdavid, sbegaudeau, AxelRICHARD and gcoutable as code owners April 29, 2024 13:54

AxelRICHARD added pr: to review soon priority: high labels Apr 29, 2024

AxelRICHARD added this to the 2024.5.0 milestone Apr 29, 2024

AxelRICHARD linked an issue Apr 29, 2024 that may be closed by this pull request

Add a diagram filter view in the diagram panel #3429

Open

1 task

gdaniel force-pushed the gda/enh/diagramFilter branch 2 times, most recently from 87b9db0 to 22ccf5e Compare April 29, 2024 13:59

gdaniel commented Apr 29, 2024

View reviewed changes

.../org/eclipse/sirius/web/application/views/explorer/services/ExplorerDescriptionProvider.java Outdated Show resolved Hide resolved

AxelRICHARD reviewed Apr 29, 2024

View reviewed changes

gdaniel force-pushed the gda/enh/diagramFilter branch 2 times, most recently from 7ec2a29 to 516b195 Compare April 30, 2024 13:31

sbegaudeau requested changes May 7, 2024

View reviewed changes

gdaniel force-pushed the gda/enh/diagramFilter branch 2 times, most recently from 148fcd0 to 5eeb50e Compare May 14, 2024 10:41

[3429] Add diagram filter view in diagram panel

35bc7d5

Bug: eclipse-sirius#3429 Signed-off-by: Gwendal Daniel <gwendal.daniel@obeosoft.com>

gdaniel force-pushed the gda/enh/diagramFilter branch from 5eeb50e to 35bc7d5 Compare May 14, 2024 12:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3429] Add diagram filter view in diagram panel #3430

[3429] Add diagram filter view in diagram panel #3430

gdaniel commented Apr 29, 2024

sbegaudeau left a comment

sbegaudeau May 7, 2024

gdaniel May 13, 2024

sbegaudeau May 7, 2024

gdaniel May 14, 2024

sbegaudeau May 7, 2024

gdaniel May 14, 2024

sbegaudeau May 7, 2024

gdaniel May 14, 2024

sbegaudeau May 7, 2024

gdaniel May 14, 2024

sbegaudeau May 7, 2024

gdaniel May 13, 2024

sbegaudeau May 7, 2024

gdaniel May 13, 2024

sbegaudeau May 7, 2024

gdaniel May 13, 2024

sbegaudeau May 7, 2024

gdaniel May 13, 2024

sbegaudeau May 7, 2024

gdaniel May 13, 2024

[3429] Add diagram filter view in diagram panel #3430

Are you sure you want to change the base?

[3429] Add diagram filter view in diagram panel #3430

Conversation

gdaniel commented Apr 29, 2024

Pull request template

General purpose

Project management

Architectural decision records (ADR)

Dependencies

Frontend

General purpose

Typing

Backend

General purpose

Architecture

Review

How to test this PR?

sbegaudeau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment