[3982] Add support for command palette #4011

gcoutable · 2024-09-17T16:25:19Z

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?

Please describe here the various use cases to test this pull request

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

Bug: #3982 Signed-off-by: Stéphane Bégaudeau <[email protected]> Signed-off-by: Guillaume Coutable <[email protected]>

sbegaudeau · 2024-10-02T09:03:23Z

CHANGELOG.adoc

@@ -31,7 +31,8 @@ Both `IInput` and `IDomainEvent` implement `ICause` and will thus be used to ind
 - https://github.com/eclipse-sirius/sirius-web/issues/3972[#3972] [diagram] The `InsideLabelStyle#displayHeaderSeparator` has been renamed to `headerSeparatorDisplayMode` and is no longer a boolean but an enum with three possible values: NEVER, ALWAYS, and IF-CHILDREN.
 Previously, the value false was equivalent to NEVER, and true to IF-CHILDREN.
 The new option ALWAYS allows the separator to be displayed in every case.
- https://github.com/eclipse-sirius/sirius-web/issues/3678[#3678] [core] Remove `IRepresentationMetadataSearchService#findByRepresentation`, use `IRepresentationMetadataSearchService#findByRepresentationId` instead. 
+- https://github.com/eclipse-sirius/sirius-web/issues/3678[#3678] [core] Remove `IRepresentationMetadataSearchService#findByRepresentation`, use `IRepresentationMetadataSearchService#findByRepresentationId` instead.
+- https://github.com/eclipse-sirius/sirius-web/issues/3982[#3982] [sirius-web] The edit project view needs to declare the `OmniboxProvider` to benefit from the command palette.


How is that a breaking change?

sbegaudeau · 2024-10-02T09:04:09Z

packages/core/backend/sirius-components-collaborative/pom.xml

+		<dependency>
+			<groupId>com.graphql-java</groupId>
+			<artifactId>graphql-java</artifactId>
+		</dependency>


Why? The collaborative part is supposed to be independent from that unless I'm mistaken

sbegaudeau · 2024-10-02T09:06:42Z

...orative/src/main/java/org/eclipse/sirius/components/collaborative/dto/PageInfoWithCount.java

@@ -10,7 +10,7 @@
 * Contributors:
 *     Obeo - initial API and implementation
 *******************************************************************************/
-package org.eclipse.sirius.web.application.dto;
+package org.eclipse.sirius.components.collaborative.dto;


Why not in org.eclipse.sirius.components.core.graphql.dto? This does not seem to be used in sirius-components-collaborative and thus the dependency to graphql-java seems useless here.

sbegaudeau · 2024-10-02T09:09:07Z

...ava/org/eclipse/sirius/components/collaborative/handlers/GetOmniboxCommandsEventHandler.java

+                    var label = this.objectService.getLabel(object);
+                    var kind = this.objectService.getKind(object);
+                    var iconURL = this.objectService.getImagePath(object);
+                    omniboxCommands.add(new OmniboxCommand(UUID.fromString(id), label, kind, iconURL, ""));


The id of the object will not be an UUID in various use cases

Add a description indicating that the element will be selected something like "Click to select the object"

sbegaudeau · 2024-10-02T09:10:14Z

...ages/core/backend/sirius-components-collaborative/src/main/resources/schema/omnibox.graphqls

+}
+input OmniboxContextEntry {
+    id: ID!
+    label: String!


The label can be removed, it will be useful on the client, not the backend

sbegaudeau · 2024-10-02T09:25:23Z

packages/diagrams/frontend/sirius-components-diagrams/package.json

+    "picomatch": "4.0.2",
+    "mini-svg-data-uri": "1.4.4",


These shouldn't be necessary

I know, I will try again without

sbegaudeau · 2024-10-02T09:25:56Z

packages/releng/backend/sirius-components-test-coverage/pom.xml

+		<dependency>
+			<groupId>org.eclipse.sirius</groupId>
+			<artifactId>sirius-components-core-graphql</artifactId>
+			<version>2024.9.1</version>


Don't forget to update it to 2024.9.3

sbegaudeau · 2024-10-02T09:26:05Z

packages/sirius-web/backend/sirius-web-starter/pom.xml

+		<dependency>
+			<groupId>org.eclipse.sirius</groupId>
+			<artifactId>sirius-components-core-graphql</artifactId>
+			<version>2024.9.1</version>


Don't forget to update it to 2024.9.3

sbegaudeau · 2024-10-02T09:27:12Z

packages/sirius-web/frontend/sirius-web/package.json

@@ -52,6 +53,7 @@
    "xstate": "4.32.1"
  },
  "devDependencies": {
+    "@rollup/plugin-image": "2.1.1",


This shouldn't be necessary for this PR

I will try again without, but maybe this change (or other dependency changes in this PR) should have been done in a previous PR.
What I mean is, sometimes, the team does not generate package-lock.json well, so the next dev who does it correctly, have to make the change that should have been done in a previous PR.

sbegaudeau · 2024-10-02T09:28:19Z

...ponents-forms/src/propertysections/__tests__/__snapshots__/ListPropertySection.test.tsx.snap

@@ -469,7 +469,7 @@ exports[`should render a readOnly list from widget properties 1`] = `
            class="MuiTableCell-root MuiTableCell-body MuiTableCell-sizeSmall css-18jhh0s-cell css-dsuxgy-MuiTableCell-root"
          >
            <p
-              class="MuiTypography-root MuiTypography-body1  css-qh6f2j-style css-1gqyz35-MuiTypography-root"
+              class="MuiTypography-root MuiTypography-body1 css-qh6f2j-style css-1gqyz35-MuiTypography-root"


Are those changes necessary? Similar question for the next file.

I had an error on test execution with the previous version (with two spaces).
It should not have been modified previously, why two spaces?
So, yes, those changes are necessary.

gcoutable added this to the 2024.11.0 milestone Sep 17, 2024

gcoutable linked an issue Sep 17, 2024 that may be closed by this pull request

Add support for a command palette #3982

Open

gcoutable added pr: to review later priority: low labels Sep 17, 2024

gcoutable force-pushed the gco/enh/add-command-palette branch 3 times, most recently from afb9817 to b7668b6 Compare September 24, 2024 09:45

gcoutable marked this pull request as ready for review September 24, 2024 09:45

gcoutable requested a review from sbegaudeau as a code owner September 24, 2024 09:45

gcoutable force-pushed the gco/enh/add-command-palette branch 2 times, most recently from b585571 to 6c226a3 Compare September 24, 2024 12:49

gcoutable requested a review from AxelRICHARD as a code owner September 24, 2024 12:49

[3982] Add support for command palette

aa444ef

Bug: #3982 Signed-off-by: Stéphane Bégaudeau <[email protected]> Signed-off-by: Guillaume Coutable <[email protected]>

gcoutable force-pushed the gco/enh/add-command-palette branch from 6c226a3 to aa444ef Compare September 24, 2024 13:38

sbegaudeau requested changes Oct 2, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3982] Add support for command palette #4011

[3982] Add support for command palette #4011

gcoutable commented Sep 17, 2024

sbegaudeau Oct 2, 2024

sbegaudeau Oct 2, 2024

sbegaudeau Oct 2, 2024

sbegaudeau Oct 2, 2024

sbegaudeau Oct 2, 2024

sbegaudeau Oct 2, 2024

gcoutable Oct 2, 2024

sbegaudeau Oct 2, 2024

sbegaudeau Oct 2, 2024

sbegaudeau Oct 2, 2024

gcoutable Oct 2, 2024

sbegaudeau Oct 2, 2024

gcoutable Oct 2, 2024 •

edited

Loading

[3982] Add support for command palette #4011

Are you sure you want to change the base?

[3982] Add support for command palette #4011

Conversation

gcoutable commented Sep 17, 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?

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

gcoutable Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

gcoutable Oct 2, 2024 •

edited

Loading