From 8429fdcef91ca720e57aa8e642878b0e976d851a Mon Sep 17 00:00:00 2001 From: Mark Sujew Date: Tue, 27 Aug 2024 15:26:57 +0200 Subject: [PATCH] Review comments --- .../browser/collaboration-color-service.ts | 1 + .../collaboration-frontend-contribution.ts | 24 +++++++++---------- .../src/browser/collaboration-instance.ts | 17 +++++-------- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/packages/collaboration/src/browser/collaboration-color-service.ts b/packages/collaboration/src/browser/collaboration-color-service.ts index a1c159cca8af7..adfac8e819118 100644 --- a/packages/collaboration/src/browser/collaboration-color-service.ts +++ b/packages/collaboration/src/browser/collaboration-color-service.ts @@ -71,6 +71,7 @@ export class CollaborationColorService { } requiresDarkFont(color: CollaborationColor): boolean { + // From https://stackoverflow.com/a/3943023 return ((color.r * 0.299) + (color.g * 0.587) + (color.b * 0.114)) > 186; } } diff --git a/packages/collaboration/src/browser/collaboration-frontend-contribution.ts b/packages/collaboration/src/browser/collaboration-frontend-contribution.ts index 40b2394eeb24c..c6e875683fe23 100644 --- a/packages/collaboration/src/browser/collaboration-frontend-contribution.ts +++ b/packages/collaboration/src/browser/collaboration-frontend-contribution.ts @@ -50,7 +50,7 @@ export const DEFAULT_COLLABORATION_SERVER_URL = 'https://oct-server-staging-ymij @injectable() export class CollaborationFrontendContribution implements CommandContribution { - protected readonly authHandlerDeferred = new Deferred(); + protected readonly connectionProvider = new Deferred(); @inject(WindowService) protected readonly windowService: WindowService; @@ -86,12 +86,12 @@ export class CollaborationFrontendContribution implements CommandContribution { url: serverUrl, client: FrontendApplicationConfigProvider.get().applicationName, fetch: window.fetch.bind(window), - opener: url => this.windowService.openNewWindow(url), + opener: url => this.windowService.openNewWindow(url, { external: true }), transports: [SocketIoTransportProvider], userToken: localStorage.getItem(COLLABORATION_AUTH_TOKEN) ?? undefined }); - this.authHandlerDeferred.resolve(authHandler); - }, err => this.authHandlerDeferred.reject(err)); + this.connectionProvider.resolve(authHandler); + }, err => this.connectionProvider.reject(err)); } protected async onStatusDefaultClick(): Promise { @@ -177,7 +177,7 @@ export class CollaborationFrontendContribution implements CommandContribution { protected async setStatusBarEntryDefault(): Promise { await this.setStatusBarEntry({ - text: '$(codicon-live-share) ' + nls.localizeByDefault('Share'), + text: '$(codicon-live-share) ' + nls.localize('theia/collaboration/collaborate', 'Collaborate'), tooltip: nls.localize('theia/collaboration/startSession', 'Start or join collaboration session'), onclick: () => this.onStatusDefaultClick() }); @@ -220,7 +220,7 @@ export class CollaborationFrontendContribution implements CommandContribution { text: nls.localize('theia/collaboration/creatingRoom', 'Creating Session'), }, () => cancelTokenSource.cancel()); try { - const authHandler = await this.authHandlerDeferred.promise; + const authHandler = await this.connectionProvider.promise; const roomClaim = await authHandler.createRoom({ reporter: info => progress.report({ message: info.message }), abortSignal: this.toAbortSignal(cancelTokenSource.token) @@ -252,7 +252,7 @@ export class CollaborationFrontendContribution implements CommandContribution { let joinRoomProgress: Progress | undefined; const cancelTokenSource = new CancellationTokenSource(); try { - const authHandler = await this.authHandlerDeferred.promise; + const authHandler = await this.connectionProvider.promise; const id = await this.quickInputService?.input({ placeHolder: nls.localize('theia/collaboration/enterCode', 'Enter collaboration session code') }); @@ -299,16 +299,16 @@ export class CollaborationFrontendContribution implements CommandContribution { navigator.clipboard.writeText(code); const notification = nls.localize('theia/collaboration/copiedInvitation', 'Invitation code copied to clipboard.'); if (firstTime) { - const makeReadonly = nls.localize('theia/collaboration/makeReadonly', 'Make readonly'); + // const makeReadonly = nls.localize('theia/collaboration/makeReadonly', 'Make readonly'); const copyAgain = nls.localize('theia/collaboration/copyAgain', 'Copy Again'); const copyResult = await this.messageService.info( notification, - makeReadonly, + // makeReadonly, copyAgain ); - if (copyResult === makeReadonly && this.currentInstance) { - this.currentInstance.readonly = true; - } + // if (copyResult === makeReadonly && this.currentInstance) { + // this.currentInstance.readonly = true; + // } if (copyResult === copyAgain) { navigator.clipboard.writeText(code); } diff --git a/packages/collaboration/src/browser/collaboration-instance.ts b/packages/collaboration/src/browser/collaboration-instance.ts index db28976f98a2f..00c89e1908a77 100644 --- a/packages/collaboration/src/browser/collaboration-instance.ts +++ b/packages/collaboration/src/browser/collaboration-instance.ts @@ -58,16 +58,8 @@ export function createCollaborationInstanceContainer(parent: interfaces.Containe return child; } -export class CollaborationPeer implements Disposable { +export interface DisposablePeer extends Disposable { peer: types.Peer; - - constructor(peer: types.Peer, protected disposable: Disposable) { - this.peer = peer; - } - - dispose(): void { - this.disposable.dispose(); - } } export const COLLABORATION_SELECTION = 'theia-collaboration-selection'; @@ -108,7 +100,7 @@ export class CollaborationInstance implements Disposable { protected readonly utils: CollaborationUtils; protected identity = new Deferred(); - protected peers = new Map(); + protected peers = new Map(); protected yjs = new Y.Doc(); protected yjsAwareness = new awarenessProtocol.Awareness(this.yjs); protected yjsProvider: OpenCollaborationYjsProvider; @@ -610,7 +602,10 @@ export class CollaborationInstance implements Disposable { const collection = new DisposableCollection(); collection.push(this.createPeerStyleSheet(peer)); collection.push(Disposable.create(() => this.peers.delete(peer.id))); - const disposablePeer = new CollaborationPeer(peer, collection); + const disposablePeer = { + peer, + dispose: () => collection.dispose() + }; this.peers.set(peer.id, disposablePeer); }