Create Room #52
Create Room #52
Conversation
…hap-web-v4 into feat/46_create_room
…hap-web-v4 into feat/46_create_room
…hap-web-v4 into feat/46_create_room
src/lib/createTchapRoom.ts
Outdated
| createRoomOpts.accessRule = TchapRoomAccessRule.Restricted; | ||
| createRoomOpts.visibility = matrixJsSdk.Visibility.Public; | ||
| createRoomOpts.preset = matrixJsSdk.Preset.PublicChat; | ||
| opts.joinRule = matrixJsSdk.JoinRule.Public; |
There was a problem hiding this comment.
Quel est l'intéret de modifier createRoomOpts sachant que ça ne sera pas utilisé de ce que je vois (ni return ni assign, seul opts est retourné) ?
There was a problem hiding this comment.
ha, je me suis posé la meme question !
En lisant de pres, un peu plus haut, tu as
const createRoomOpts: ITchapCreateRoomOpts = {};
opts.createOpts = createRoomOpts;
Et tout à la fin return opts;, du coup on a return createRoomOpts indirectement.
src/lib/createTchapRoom.ts
Outdated
| * @returns rooms options | ||
| */ | ||
| export default function roomCreateOptions(name: string, tchapRoomType: TchapRoomType, federate?: boolean): IOpts { | ||
| federate = (federate != undefined) ? federate : DEFAULT_FEDERATE_VALUE; |
There was a problem hiding this comment.
Qu'est ce que vous pensez de federate = federate ?? DEFAULT_FEDERATE_VALUE; ou même dans le constructeur federate: boolean = DEFAULT_FEDERATE_VALUE
There was a problem hiding this comment.
bonne idee ! Je met un default pour l'argument de la fonction.
src/util/TchapUtils.ts
Outdated
| /** | ||
| * Return a short value for getDomain(). | ||
| * @returns {string} The shortened value of getDomain(). | ||
| */ | ||
| static getShortDomain() { |
There was a problem hiding this comment.
Je propose d'utiliser le typing TS plutôt que JSdoc pour typer les fonctions. Le commentaire n'apporte pas grand chose en prime
There was a problem hiding this comment.
Bonne remarque, done
| expect(roomCreateOptions("testName", TchapRoomType.Private)).toStrictEqual( | ||
| { | ||
| createOpts: { | ||
| name: "testName", | ||
| creation_content: { | ||
| "m.federate": true, | ||
| }, | ||
| accessRule: "restricted", | ||
| visibility: "private", | ||
| preset: "private_chat", | ||
| }, | ||
| guestAccess: false, | ||
| joinRule: "invite", | ||
| encryption: true, | ||
| historyVisibility: "joined", | ||
| } | ||
| ) |
There was a problem hiding this comment.
En refacto possible, il serait sympa d'extraire le resultat attendu dans une variable ie. privateRoomExpectedSettings, ça permet de faire des expects plus courts et lisibles, et en prime, l'expected value est réutilisable
There was a problem hiding this comment.
allez, ca sera joli, vendu :)
webpack.config.js
Outdated
| @@ -36,6 +36,7 @@ function getActiveThemes() { | |||
| // Default to `light` theme when the MATRIX_THEMES environment variable is not defined. | |||
| const theme = process.env.MATRIX_THEMES ?? 'light'; | |||
| const themes = theme.split(',').filter(x => x).map(x => x.trim()).filter(x => x); | |||
| console.log("Active themes : ", themes) | |||
There was a problem hiding this comment.
Ben chepa vraiment, @odelcroi t'avais l'intention de le garder ? Ya pas trop de rapport avec le sujet de la PR. C'est utile ?
There was a problem hiding this comment.
Je supprime pour le moment, on le remettra si c'est utile.
…ded what to do when spaces are enabled.
|
Bon, fin du sprint, je merge ! Ya encore du travail, donc je vais mettre des todo dans l'issue correspondante. |
|
@giomfo
|
Co-authored-by: dbkr <986903+dbkr@users.noreply.github.com> (cherry picked from commit 3256499d4bacbf8cb527c1261c65f805de346c53)
#46