Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Implement updated open dialog method of the Module API #11395

Merged

Conversation

dhenneke
Copy link
Contributor

@dhenneke dhenneke commented Aug 10, 2023

Implements the changes of matrix-org/matrix-react-sdk-module-api#14
Replaces the outdated PR #10536

Companion PR: element-hq/element-web#25986

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Type: enhancement


Here's what your changelog entry will look like:

✨ Features

  • Implement updated open dialog method of the Module API (#11395). Contributed by @dhenneke.

Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>
@github-actions github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Aug 10, 2023
@github-actions github-actions bot added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Aug 10, 2023
Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>
Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>
Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>
Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

Looks good to me but I'll leave @richvdh to tick in case he has more context.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

Looks great otherwise!

@@ -61,11 +65,23 @@ export class ModuleUiDialog<P extends DialogProps, C extends DialogContent<P>> e
this.props.onFinished(false);
}

protected setOptions(options: ModuleUiDialogOptions): void {
Copy link
Member

Choose a reason for hiding this comment

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

could this be private?

contentProps: P;
title: string;
onFinished(ok?: boolean, model?: Awaited<ReturnType<DialogContent<P>["trySubmit"]>>): void;
contentProps: Omit<P, keyof DialogProps> | undefined;
Copy link
Member

Choose a reason for hiding this comment

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

wonder if this should be called additionalContentProps to distinguish it from the actual contentProps

cancel: this.cancel.bind(this),
};

// Typescript isn't very happy understanding that `contentProps` satisfies `Omit<P, keyof DialogProps>`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Typescript isn't very happy understanding that `contentProps` satisfies `Omit<P, keyof DialogProps>`
// Typescript isn't very happy understanding that `contentProps` satisfies `P`

Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

thank you!

@richvdh richvdh enabled auto-merge August 18, 2023 12:02
@dhenneke
Copy link
Contributor Author

Does GitHub has issues?

image

@richvdh
Copy link
Member

richvdh commented Aug 18, 2023

No, it needs a review from someone on @matrix-org/element-web-app-team due to the changes in package.json.

@richvdh richvdh requested a review from a team August 18, 2023 15:57
@richvdh richvdh added this pull request to the merge queue Aug 21, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Aug 21, 2023
@dhenneke
Copy link
Contributor Author

I resolved the merge conflict.

@richvdh richvdh enabled auto-merge August 21, 2023 09:27
@dhenneke dhenneke requested a review from richvdh August 21, 2023 10:17
@richvdh richvdh added this pull request to the merge queue Aug 21, 2023
Merged via the queue into matrix-org:develop with commit 5c43054 Aug 21, 2023
16 checks passed
@dhenneke dhenneke deleted the nic/feat/module-api-dialog-improvements branch August 21, 2023 12:57
andybalaam pushed a commit that referenced this pull request Aug 21, 2023
* Implement updated open dialog method

Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>

* Apply the review comments

Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>

* Add unit tests for the module system dialog

Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>

* Bump @matrix-org/react-sdk-module-api from 1.0.0 to 2.0.0

Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>

* Run prettier

Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>

* Apply review comments

Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>

---------

Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>
@Johennes Johennes added A-Timesheet-1 Log any time spent on this into the A-Timesheet-1 project and removed Z-Community-PR Issue is solved by a community member's PR labels Aug 24, 2023
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 31, 2023
Changes in [1.11.40](https://github.com/vector-im/element-web/releases/tag/v1.11.40) (2023-08-29)
=================================================================================================

## ✨ Features
 * Hide account deactivation for externally managed accounts ([\#11445](matrix-org/matrix-react-sdk#11445)). Fixes #26022. Contributed by @kerryarchibald.
 * OIDC: Redirect to delegated auth provider when signing out ([\#11432](matrix-org/matrix-react-sdk#11432)). Fixes #26000. Contributed by @kerryarchibald.
 * Disable 3pid fields in settings when `m.3pid_changes` capability is disabled ([\#11430](matrix-org/matrix-react-sdk#11430)). Fixes #25995. Contributed by @kerryarchibald.
 * OIDC: disable multi session signout for OIDC-aware servers in session manager ([\#11431](matrix-org/matrix-react-sdk#11431)). Contributed by @kerryarchibald.
 * Implement updated open dialog method of the Module API ([\#11395](matrix-org/matrix-react-sdk#11395)). Contributed by @dhenneke.
 * Polish & delabs `Exploring public spaces` feature ([\#11423](matrix-org/matrix-react-sdk#11423)).
 * Treat lists with a single empty item as plain text, not Markdown. ([\#6833](matrix-org/matrix-react-sdk#6833)). Fixes element-hq/element-meta#1265.
 * Allow managing room knocks ([\#11404](matrix-org/matrix-react-sdk#11404)). Contributed by @charlynguyen.
 * Pin the action buttons to the bottom of the scrollable dialogs ([\#11407](matrix-org/matrix-react-sdk#11407)). Contributed by @dhenneke.
 * Support Matrix 1.1 (drop legacy r0 versions) ([\#9819](matrix-org/matrix-react-sdk#9819)).

## 🐛 Bug Fixes
 * Fix path separator for Windows based systems ([\#25997](element-hq/element-web#25997)).
 * Fix instances of double translation and guard translation calls using typescript ([\#11443](matrix-org/matrix-react-sdk#11443)).
 * Fix export type "Current timeline" to match its behaviour to its name ([\#11426](matrix-org/matrix-react-sdk#11426)). Fixes #25988.
 * Fix Room Settings > Notifications file upload input being shown superfluously ([\#11415](matrix-org/matrix-react-sdk#11415)). Fixes #18392.
 * Simplify registration with email validation ([\#11398](matrix-org/matrix-react-sdk#11398)). Fixes #25832 #23601 and #22297.
 * correct home server URL ([\#11391](matrix-org/matrix-react-sdk#11391)). Fixes #25931. Contributed by @NSV1991.
 * Include non-matching DMs in Spotlight recent conversations when the DM's userId is part of the search API results ([\#11374](matrix-org/matrix-react-sdk#11374)). Contributed by @mgcm.
 * Fix useRoomMembers missing updates causing incorrect membership counts ([\#11392](matrix-org/matrix-react-sdk#11392)). Fixes #17096.
 * Show error when searching public rooms fails ([\#11378](matrix-org/matrix-react-sdk#11378)).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Timesheet-1 Log any time spent on this into the A-Timesheet-1 project T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants