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.

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

[Sprint Goal] Avancer Tartine : design technique d'une feature #46

Closed
celiacheff opened this issue Apr 12, 2022 · 7 comments
Closed

[Sprint Goal] Avancer Tartine : design technique d'une feature #46

celiacheff opened this issue Apr 12, 2022 · 7 comments
Assignees
Labels

Comments

@celiacheff
Copy link
Contributor

celiacheff commented Apr 12, 2022

Faire le design technique d’une feature à choisir

Objectif bonus : implémentation

Le design doit être fait avec précautions, pour éviter le plus possible de modifier Elément. Checker avec Travis qu’on utilise la bonne méthode.

@celiacheff celiacheff added P1 Priority 1 v4 web labels Apr 12, 2022
@estellecomment estellecomment changed the title [Sprint Goal] Avancer Tartine [Sprint Goal] Avancer Tartine : design technique d'une feature Apr 12, 2022
@estellecomment
Copy link
Contributor

estellecomment commented Apr 12, 2022

Une feature simple pour commencer : création de room.

  • où est le composant existant à remplacer, dans le code element ?
  • quel code on va écrire ? Quels appels serveur, quel composant, etc (s'inspirer du code tchap de v2, et de element)
  • quelle methode de customisation utiliser ? (webpack file replacement, customisations, autre ? Checker avec Travis)
  • Optionnel : les externes. Est-ce qu'on peut faire un truc temporaire à la place ? Ou on les ignore pour le moment.
    • les externes ne peuvent pas créer de room
    • il faut pouvoir creer des rooms avec ou sans externes.

@celiacheff
Copy link
Contributor Author

celiacheff commented Apr 13, 2022

WIP:
J'ai effectué une recherche sur le sujet, et testé un peu le component. Ce que j'ai appris:

Component incriminé (et autres fichiers utiles)

v4 et en version js en v2:

  • le composant: matrix-react-sdk/src/components/views/dialogs/CreateRoomDialog.tsx
  • matrix-react-sdk/src/createRoom.ts
  • matrix-js-sdk/src/client.ts

Code à écrire

  • Le code de v2 est assez simple, mais un gros rafraîchissement serait bienvenue (on peut simplifier la liste de params envoyés au serveur - federate par exemple, etc...
  • En appel serveur, c'est des variations de la fonction L98 en v2. Le onFinished de la fonction est définie en V2 dans MatrixChat.js L972. Il y a donc une modif sur le client, en réalité que pour les externes avec le flag isExtern, mais je n'ai pas cherché assez loin pour voir si on peut s'en passer au final (a checker avec les ops, et avoir mes remarques sur les externes ci-dessous).
  • Sur V4, le code est dans MatrixChat.tsx L1034, à noter que comme déjà en v2 le createRoom possède son propre fichier createRoom.ts, ce qui est sympa pour des overrides localisé (le code est bien plus propre).

Customisation

  • Testé comme le custom server picker
  • autres options disponibles:
    • customisation.json (juste du webpack sous le capot)
    • modules Element (dont on est sans infos)
  • J'ai aussi pas encore testé la possibilité de faire de l'héritage sur un component (au lieu de repartir de 0, faire un truc comme TchapCreateRoomDialog extends CreateRoomDialog et du coup on peut partager du code en commun et on défini uniquement ce que l'on veut changer.
  • Aussi la possibilité de faire ça avec carrément le MatrixClient pour que l'on puisse le customiser sans toucher au js-sdk

Externes

  • Si on modifie tout le component, on peut simplement checker si l'utilisateur a dans le HS 'e.' ou 'externe.' comme sur la V2. Mais il faudrait vraiment avoir des vrais ACL, ou un flag.
  • On a aussi la possibilité de masquer facilement le component pour les externes en rajoutant le petit bout de code a la fin de https://github.com/vector-im/element-web/blob/develop/docs/customisations.md
  • Je me demande aussi si on pourrait pas simplement écraser le setGuest dans client.ts pour renvoyer notre définition d'un guest ( un externe au lieu de juste un HS différent) facilement (mais j'ai pas eu le temps de vérifier toutes les implications qui pourraient être peut être très positives, surtout cumulées avec les options/restrictions standard d'Element pour les guests) (ou négatives si y'a des effets secondaires :/). Il me semble qu"on a tout de même des notions de guests qui sont pas des externes dans nos flows, mais je n'en suis pas certain, à vérifier avec Giom.

@celiacheff celiacheff self-assigned this Apr 14, 2022
@estellecomment
Copy link
Contributor

Brain dump de Giom sur ce sujet, dans https://www.notion.so/Diff-rences-entre-Element-et-Tchap-329cf4d20ce840a3b53e29dd2d76f0f0#f54d6d57743d497aafce90935e0c0044.
Je nettoie mes notes pour vous les transmettre.

@estellecomment
Copy link
Contributor

estellecomment commented Apr 15, 2022

Details d'implementation (pas forcément exhaustif):

  • Notre dropdown choisira TchapRoomType (comme celui d'android), alors que celui d'element choisit JoinRule.

  • Les options qu'on devra spécifier au moment de la room creation sont dans CreateRoomDialog.tsx, this.roomCreateOptions()
    Les options passées par element :

    • name : le nom du channel
    • Topic : on en a pas ?
    • Join_rule : public, invite, restricted
    • noFederate : false (tchap veut tjs federer, c’est a dire autoriser les users de tous les homeservers)
    • parentSpace : on touche à rien (sauf peut etre avec le join_rule ?)
    • associatedWithCommunity : les communities sont bientot deprecated, replacees par les spaces, on y touche pas

Version Tchap :

  • Join_rule : salon=invite, forum=public.
  • Access_rule : pour salon sans externe et forum : restricted. Pour salon avec externes : unrestricted
  • Encryption : true pour salon, false pour forum
  • Preset : public_chat pour forum (ca met guest_access=true, mais on peut le override)
  • History_visibility : shared pour forum, joined ou invited (?) pour salon
  • Guest_access : disable partout

Overall je suis pas convaincue que preset soit tres utile, ca ajoute de la confusion, puisqu’on l’override ensuite (mais peut etre que ca rajoute des options utiles plus tard ?).

@estellecomment estellecomment self-assigned this Apr 15, 2022
@estellecomment
Copy link
Contributor

Les points de customisations proposées par element n'ont pas l'air de couvrir ce qu'on veut. Donc je pense pas qu'on pourra utiliser customisations.

Du coup, c'est webpack file replacement, il reste que ca (puisque replaceableComponent est deprecated)

@estellecomment
Copy link
Contributor

estellecomment commented Apr 15, 2022

Précision sur domain et federate :

  • les salons sont tjs federated=true (c'est a dire les users de toutes les instances peuvent les rejoindre)
  • les forums laissent federated au choix de l'utilisateur : on peut limiter à l'instance de l'utilisateur, ou non.
    • exception pour les utilisateurs de l'instance agent qui n'ont que des forums federated (??? a verifier) Edit : parce que les users de "agent" ne comprenaient pas l'option, à cause du nom "agent" qui n'est pas clair, et apres ils contactaient le support parce que ca ne faisait pas ce qu'ils pensaient.

@estellecomment
Copy link
Contributor

Première version mergée !
Reste à faire :

  • Undo les changements d'interface sur le toggle de federation. Pour décider de changer, il faudrait faire des user tests pour vérifier que c'est utile, et aligner tous les clients sur le changement. (discussion en cours là : https://github.com/tchapgouv/tchap-design/issues/73) Donc pour le moment on revient à l'identique de v2.
  • Le comportement spécifique de l'instance agent (ou intradef pour le moment) est hard-codé. Ca permet pas de tester en preprod par exemple, puis que ca ne marche que pour le domaine agent.tchap.gouv.fr. (PR draft en cours [refactor] Get room federation options from config #59)
  • Unit-tester le nouveau composant. On a testé que la logique métier pour le moment. Faudrait tester le composant pour vérifier que quand on clique les boutons, ca fait bien ce qu'on pense.
  • Faire tourner les tests de matrix-react-sdk (Olivier est train de les patouiller je crois)

On a pas pris en compte les spaces, et si il faut des options spéciales (par ex "Visible to users in the space"). C'est hors-scope parce qu'on sait pas vraiment encore ce que fera Tchap pour les spaces.

@odelcroi odelcroi removed the P1 Priority 1 label Jun 7, 2022
MarcWadai pushed a commit that referenced this issue Oct 8, 2024
…9d90..892a9e658e8

892a9e658e8 Merge pull request #11 from tchapgouv/upgrade-v3.112.0
7cf9e10e21d Fix merge conflict v3.112.0
ebd1e11d284 Merge tag 'v3.112.0' into upgrade-v3.112.0
933885386ed v3.112.0
e4ed18297cf Upgrade dependency to matrix-js-sdk@34.7.0
3687ee382dd test(sso): add test for email precheck, registration and welcome component
b32de09a709 v3.112.0-rc.0
08b3912de0f Upgrade dependency to matrix-js-sdk@34.7.0-rc.0
dd405ea582d Merge remote-tracking branch 'origin/develop' into staging
f33e802 Fix untranslated keys being rendered in `/help` dialog (#90)
cd850f5be67 v3.111.0
059db4db1b8 Merge pull request #106 from element-hq/backport-95-to-staging
d30645f9084 Allow joining calls and video rooms without enabling the labs flags (#95)
39a0f6e Remove ts-ignores where no longer necessary (#89)
c10bc6c7168 feat(sso): add feature flag to active sso flow
36fae00 Change device isolation mode to set `errorOnVerifiedUserProblems` to `false` (#104)
33c900e Remove right panel toggling behaviour on room header buttons (#100)
81bb56a Simplify Jest runs in CI to share failures with merge queue (#103)
fe402e2 Fix flaky mobile registration tests (#102)
0b3b499 Fix label sync (#101)
bd793a0 Allow joining calls and video rooms without enabling the labs flags (#95)
4f39164 Ensure timeline search results are visible even in video rooms (#96)
f28f1d9 Improve error display for messages sent from insecure devices (#93)
be2c1fc Add labs option to exclude unverified devices (#92)
78cca0201a9 feat(sso): remove existing sso buttons
8962e8c Improve contrast for timestamps, date separators & spotlight trigger (#91)
ef9e310 Pop right panel timeline when unmaximising widget to avoid double timeline (#94)
fed6c34d440 feat(sso): add email domain precheck sso flow
81192f6 Update dependency typescript to v5.6.2 (#71)
34d1875 Open room settings on room header avatar click (#88)
3f67819 Merge pull request #41 from element-hq/t3chguy/wat/230.1
e6404da Update test assertions
9e4348e Update test assertions
dd7479a Merge branch 'develop' of github.com:element-hq/matrix-react-sdk into t3chguy/wat/230.1
ad94c39 Fix accessible label on left panel spotlight trigger (#87)
2e895da Crypto: fix display of device key (#86)
df9d81398ab v3.110.0
1d5d0cc835f Upgrade dependency to matrix-js-sdk@34.6.0
3c267f9 Update snapshots
3620c5a Merge branch 'develop' into t3chguy/wat/230.1
a1bdcee Update dependency @types/node to v18.19.50 (#65)
ef1d4f6 Grant Element Call widget capabilities for "raise hand" feature (#82)
47a9377 Update dependency @types/react to v17.0.82 (#66)
9aa09d4 Maybe fix flakey AddRemoveThreepid test (#81)
d56b9ed Update dependency eslint to v8.57.1 (#68)
a248788 Update peter-evans/create-pull-request digest to 5e91468 (#64)
1f55710 Mobile registration optimizations and tests (#62)
4be5338 Update dependency @sentry/browser to v8.30.0 (#69)
b055908 Update dependency css-tree to v3 (#74)
d4c942d Update playwright monorepo to v1.47.1 (#73)
ed7e02a Update dependency stylelint-scss to v6.6.0 (#70)
5058d66 Update Sibz/github-status-action digest to faaa4d9 (#63)
cf8fe20 Update dependency express to v4.20.0 [SECURITY] (#26)
fe65702 Update to use non deprecated methods to decode recovery key (#54)
490746e Update to use non deprecated methods to derive key from passphrase (#55)
73843e5fe6b v3.110.0-rc.1
4776f87 Ignore chat effect when older than 48h (#48)
0cc0ebe Replace old reference of `matrix-org/matrix-react-sdk` by `element-hq/matrix-react-sdk` (#60)
3dd223c Also add NPM_TOKEN (#57)
1e76313 Playwright: factor out some common code (#49)
154bf33 Manually clear orphaned recaptcha challenge overlay in android webviews (#53)
d04d611f1e8 v3.110.0-rc.0
0bbed85 Pass bot token through explicitly (#56)
6eb332f26d5 Upgrade dependency to matrix-js-sdk@34.6.0-rc.0
49d84a6b0f9 Merge remote-tracking branch 'origin/develop' into staging
7feb5a0 Merge branch 'develop' into t3chguy/wat/230.1
13e67ae Add Release announcement for the pinning message list (#46)
1058af6 Playwright test for E2E messages from deleted devices (#47)
78ac691f50b feat(settings): remove generalusersettingtab and use accountusersetting tabs, hide personal info
c24661f [create-pull-request] automated change (#45)
5985277 Unlabs feature pinning (#22)
74885c9 Merge pull request #42 from element-hq/langleyd/mobile_registeration
20a4f0a Enforce config setting
3d89fc3 Merge branch 'develop' of https://github.com/element-hq/matrix-react-sdk into langleyd/mobile_registeration
62d66f9 Remove accidental paste
0716434 Allow hs_url as param on mobile_register
a89f61a Add error text and title with server name
9426fec Fix timeout type (#40)
be59791 Add support for `org.matrix.cross_signing_reset` UIA stage flow (#34)
8044ce4 Fix tests
b505828 update test to work with newer Rust crypto (#32)
0244aae use window.dispatchEvent
a6dec86 Add mobile registration and dispatch event for mobile postmessage dance
a6e98b0 Use `strong` over `b` for improved a11y semantics
6b384fe Fix huge usage bandwidth and performance issue of pinned message banner. (#37)
5740bdb [create-pull-request] automated change (#39)
0e8cd5b [create-pull-request] automated change (#38)
eae9d9e Add timezone to user profile (#20)
f317763 Reverse pinned message list (#19)
85b4f17 [create-pull-request] automated change (#33)
a701e3a Add config option to force verification (#29)
75918f5 Reduce pinned message banner size (#28)
433c14e Log clearer errors when picklekey goes missing (#27)
d337fba Add labels file (#21)
03004a5 Change settings to true by default (#25)
07125f5 Remove release announcement of new header (#23)
ccb1a61 [create-pull-request] automated change (#24)
51495e7 Remove pinned message list screenshot which are flacky (#17)
24fe2f2 [create-pull-request] automated change (#18)
491f0cd Change license (#13)
4382c67 Change org to element-hq (#7)
461da98 Merge pull request #10 from element-hq/dbkr/changelog_repos
e8c0b65 Merge branch 'develop' into dbkr/changelog_repos
6dd67b3 Merge pull request #11 from element-hq/dbkr/update_org_sonar
c26ce7a Merge branch 'develop' into dbkr/changelog_repos
3a42d2a Merge branch 'develop' into dbkr/update_org_sonar
d4771dd Merge pull request #3 from element-hq/dbkr/codeowners
58331fb Merge pull request #15 from element-hq/actions/localazy-download
0acc7dd [create-pull-request] automated change
272fd75 remove meangingless change
9fb871c poke the CI
caa6f26 Merge pull request #14 from element-hq/actions/playwright-image-updates
92b3eb5 [create-pull-request] automated change
deeeffa Update project key too
35f96b4 Update GH org in the sonar config file
9601be5 Update tests
f8da257 Update repos for changelog fetching
774222f Update codeowners to element-hq teams
78059e1 Merge pull request #1 from element-hq/actions/playwright-image-updates
eb14223 [create-pull-request] automated change
33791ca Merge pull request #12965 from matrix-org/florianduros/pinned-messages/analytics-event
08d1b6c Add analytics event for pinned messages
bce710e Upgrade `@matrix-org/analytics-events` to `0.25.0`
5bfbca9 Migrate all pinning checks and actions into `PinningUtils` (#12964)
2639923 Update browserslist (#12953)
6490742 Log phases in the verification process (#12963)
cdffbdb Add error handling for room publish toggle (#12941)
1e3320d Pinned message list: prevent sender name to overflow pinned event tile (#12947)
ab1e28b Compute with of content of pinned event tile (#12951)
60fe70b Add a prefix to file, poll, image, video and audio in the pinned message banner (#12950)
9d8c5b6 Update dependency @testing-library/jest-dom to v6.5.0 (#12957)
dbc8c9f Update peter-evans/create-pull-request action to v7 (#12960)
33404e4 Update stylelint (#12958)
df82c8a Update dependency @sentry/browser to v8.27.0 (#12956)
892b297 Update babel monorepo (#12955)
4769985 Update dependency @types/node to v18.19.47 (#12954)
8f22eb64432 v3.109.0-rc.0
9c233ef5db3 Upgrade dependency to matrix-js-sdk@34.5.0-rc.0
6bfdb3e Fix read receipt animation (#12923)
5ff3fd6 [create-pull-request] automated change (#12949)
579cb6b Update to 2.37.9 (#12943)
f033b64 Display the indicator even with one message in pinned message banner (#12946)
41686bb Always display last pinned message on the banner (#12945)
1ac533e Don't emit decrypted event for the banner (#12944)
ae15bbe Allow user to set timezone (#12775)
acc7342 [create-pull-request] automated change (#12942)
3d41f5b [create-pull-request] automated change (#12938)
13ec19c Sort the pinning message list in the same order than the banner (#12937)
19f8b44 Implement download_file in widget driver (#12931)
2a450c0 Add `allowImportingTsExtensions` to tsconfig (#12939)
d16ab09 Display pinned messages on a banner at the top of a room (#12917)
8b2ded8 [create-pull-request] automated change (#12935)
6fb8f6e Update all non-major dependencies (#12909)
c6922c9 Fix reply message truncation on 2 lines (#12929)
ea3c5cf Fix pin/unpin slowness and non refresh from the message action bar (#12934)
43941ef Install deja-vu font in docker image (#12932)
71c31bb [create-pull-request] automated change (#12926)
8421022 Rename all the slow reporter stuff to cjs (#12933)
5b91dd8 Reset matrix-js-sdk back to develop branch
e66807e Merge branch 'master' into develop
30f84cd Update playwright image (#12930)
f0a75d8 Add a config option to control the default widget container height (#12893)
e599428 Ignore desktop for minimum browser support. (#12928)
1b70b22 Update typescript-eslint monorepo to v7.18.0 (#12924)
8381e13 Update stylelint (#12922)
5a9d7ba Remove unused CryptoCallbacks implementations (#12919)
69da175 Update babel monorepo (#12920)
0848237 Update dependency @types/sanitize-html to v2.13.0 (#12921)
135d94c Update playwright monorepo to v1.46.1 (#12918)
670ed81 Update dependency @sentry/browser to v8.26.0 (#12915)
35fb068 Update dependency axe-core to v4.10.0 (#12916)
9671545 Update dependency eslint-plugin-unicorn to v55 (#12913)
5e56ce7 Update dependency husky to v9 (#12914)
70665d3 RTE drafts (#12674)
fdc5acd Update dependency @types/react-transition-group to v4.4.11 (#12912)
72d5659 Update dependency @types/node to v18.19.44 (#12911)
2768b9c Set entrypoints to use ./lib rather than ./src (#12906)
a7e907e Add thread information in pinned message list (#12902)
3d80eff Add Pin/Unpin action in quick access of the message action bar (#12897)
4064db1 [create-pull-request] automated change (#12907)
933a9c1 Rename prettier config file to .cjs (#12903)

git-subtree-dir: linked-dependencies/matrix-react-sdk
git-subtree-split: 892a9e658e8a3f6fd225d2104b0e5001cd12ad7f
MarcWadai pushed a commit that referenced this issue Dec 2, 2024
* Add RA for the pinning message list

* Update RoomSummaryCard-test.tsx snapshot

* Update RA labels
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants