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.

Sign up for GitHub

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

Jump to bottom

[4403] Add support of actions in table rows context menu #4405

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[4403] Add support of actions in table rows context menu #4405

wants to merge 2 commits into from

Conversation

jerome-obeo
Copy link
Contributor

Bug: #4403

Pull request template

General purpose

What is the main goal of this pull request?

  • Bug fixes
  • New features
  • Documentation
  • Cleanup
  • Tests
  • Build / releng

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

  • Variables have a proper type
  • Functions’ arguments have a proper type
  • Functions’ return type are specified
  • Hooks are properly typed:
    • useMutation<DATA_TYPE, VARIABLE_TYPE>(…)
    • useQuery<DATA_TYPE, VARIABLE_TYPE>(…)
    • useSubscription<DATA_TYPE, VARIABLE_TYPE>(…)
    • useMachine<CONTEXT_TYPE, EVENTS_TYPE>(…)
    • useState<STATE_TYPE>(…)
  • All components have a proper typing for their props
  • No useless optional chaining with ?. (if the GraphQL API specifies that a field cannot be null, do not treat it has potentially null for example)
  • Nullable values have a proper type (for 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

  • 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?

@frouene frouene added this to the 2025.2.0 milestone Jan 13, 2025
@frouene frouene linked an issue Jan 13, 2025 that may be closed by this pull request
Add support of actions in table rows context menu #4403
Open
1 task
frouene
frouene requested changes Jan 14, 2025
View reviewed changes
packages/tables/frontend/sirius-components-tables/src/rows/RowContextMenu.tsx Outdated Show resolved Hide resolved
packages/tables/frontend/sirius-components-tables/src/rows/useRowContextMenuEntries.ts Outdated Show resolved Hide resolved
packages/tables/frontend/sirius-components-tables/src/rows/useRowContextMenuEntries.types.ts Outdated Show resolved Hide resolved
packages/tables/frontend/sirius-components-tables/src/rows/useRowContextMenuEntries.types.ts Outdated Show resolved Hide resolved
packages/tables/frontend/sirius-components-tables/src/rows/RowContextMenu.tsx Outdated Show resolved Hide resolved
packages/tables/frontend/sirius-components-tables/src/rows/RowContextMenu.types.ts Outdated Show resolved Hide resolved
packages/tables/frontend/sirius-components-tables/src/rows/RowContextMenu.types.ts
@@ -1,5 +1,5 @@
/*******************************************************************************
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github indicates that this file is moved from ResizeRowHandler (that thus does not exist anymore), is it normal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResizeRowHandler has been removed from the table/row folder since it has been copied into rows.
RowContextMenu.types.ts is a new file.

packages/tables/frontend/sirius-components-tables/src/table/row/ResizeRowHandler.tsx
@@ -1,73 +0,0 @@
/*******************************************************************************
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why deleted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was already copied in rows folder but not removed at this time.

packages/tables/frontend/sirius-components-tables/src/table/TableContent.tsx
@@ -126,9 +127,9 @@ export const TableContent = memo(
onPaginationChange(pagination.cursor, pagination.direction, pagination.size);
}, [pagination.cursor, pagination.size, pagination.direction]);

const handleRowHeightChange = (rowId, height) => {
function handleRowHeightChange(rowId, height) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is to avoid massive code changes.
This function is now passed as a parameter of useTableColumns located at the beginning of the component.
It would have been moved before it was passed to useTableColumns and bring with it other codes.

...bles/backend/sirius-components-collaborative-tables/src/main/resources/schema/table.graphqls Outdated Show resolved Hide resolved
@jerome-obeo jerome-obeo force-pushed the jgo/enh/4403_add_actions_in_table_rows_context_menu branch 5 times, most recently from 3aee130 to 34b2e24 Compare January 15, 2025 08:49
@jerome-obeo jerome-obeo requested a review from pcdavid as a code owner January 15, 2025 08:49
@jerome-obeo jerome-obeo force-pushed the jgo/enh/4403_add_actions_in_table_rows_context_menu branch 2 times, most recently from 3d1b58e to bb65198 Compare January 15, 2025 09:45
@jerome-obeo jerome-obeo force-pushed the jgo/enh/4403_add_actions_in_table_rows_context_menu branch from bb65198 to b6f2a74 Compare January 15, 2025 12:19
@frouene frouene force-pushed the jgo/enh/4403_add_actions_in_table_rows_context_menu branch from b6f2a74 to da1a252 Compare January 15, 2025 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frouene frouene frouene requested changes

@sbegaudeau sbegaudeau Awaiting requested review from sbegaudeau sbegaudeau is a code owner

@pcdavid pcdavid Awaiting requested review from pcdavid pcdavid is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
pr: to review soon priority: low
Projects
None yet
Milestone
2025.2.0
Development

Successfully merging this pull request may close these issues.

Add support of actions in table rows context menu
2 participants
@jerome-obeo @frouene