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

Implemented /join #809

Merged
merged 22 commits into from
Jul 29, 2022
Merged

Implemented /join #809

merged 22 commits into from
Jul 29, 2022

Conversation

Kaki-In
Copy link
Contributor

@Kaki-In Kaki-In commented Jul 21, 2022

Implemented the /join command.

Change that have been added

  • add a method with a switch syntax to improve the commands manager.
  • add the /join command - we can now join a room using /join #roomname:server.addr
  • improve the error message display (css)

Should fix this issue (#792).

…al commands uses, and some others commands like /shrug .)
Copy link
Contributor

@MidhunSureshR MidhunSureshR left a comment

Choose a reason for hiding this comment

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

Looking good, leaving some comments here:

src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
@bbhtt bbhtt mentioned this pull request Jul 24, 2022
@Kaki-In Kaki-In marked this pull request as draft July 25, 2022 11:21
…id the synchronisations waits that can sometimes disable to enter the room (message "You're not into this room" or simply "You're not in this room yet. *Join the room*")
Copy link
Contributor

@MidhunSureshR MidhunSureshR left a comment

Choose a reason for hiding this comment

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

Some more comments:

src/platform/web/ui/session/room/RoomView.js Outdated Show resolved Hide resolved
src/matrix/Session.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/matrix/Session.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/matrix/Session.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
@bwindels bwindels marked this pull request as ready for review July 25, 2022 12:55
into RoomViewModel._processCommands
If the /join command success, an error was thrown, because of a copy-pasted command not well integrated
The button of the error on "theme.css" contains now an unicode cross. The :after/:before cross was disformed when opening the room informations.
 - renamed executeJoinCommand as joinRoom;
 - separated the joinRoom process and the parse and result process
@Kaki-In
Copy link
Contributor Author

Kaki-In commented Jul 27, 2022

I cannot send any message with this implement, it doesn't throw any error, I can see the ensureMessageKeyIsShared message on the console log, but nothing is sent...
With master it works successfully.

I often have this error :
A listener indicated an asynchronous response by returning true, but the message channel closed before a response was received

:3000/#/session/1692…bGbqHQR:discu.top:1 Uncaught (in promise) {message: 'A listener indicated an asynchronous response by r…age channel closed before a response was received'}
Promise.then (async)
b @ content_script_bundle.js:100
js @ content_script_bundle.js:56
t.runContentScripts @ content_script_bundle.js:163
(anonymous) @ content_script_bundle.js:71
Promise.then (async)
t.default @ content_script_bundle.js:71
1366 @ content_script_bundle.js:9
n @ content_script_bundle.js:1
1365 @ content_script_bundle.js:1
n @ content_script_bundle.js:1
1364 @ content_script_bundle.js:1
n @ content_script_bundle.js:1
1364 @ content_script_bundle.js:1
(anonymous) @ content_script_bundle.js:1

I absolutely don't know why this probleme occurs, I have read all my code again and if the first conditions are not entered by a simple message, I don't understand why the message isn't sent.

 - fixed a pretty syntax miss (a !== b);
 - fixed a type error : replaced "msgtype" by "type" when instantied the "messinfo" variable;
 - some indentation fixes
Copy link
Contributor

@MidhunSureshR MidhunSureshR left a comment

Choose a reason for hiding this comment

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

Last couple of changes:

src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/domain/session/room/RoomViewModel.js Outdated Show resolved Hide resolved
src/platform/web/ui/css/themes/element/theme.css Outdated Show resolved Hide resolved
 - joined the processJoinRoom and joinRoom methods;
 - fixed some precisions miss;
 - removed some useless code;
 - change the error message height from absolute (40px) to relative (auto)
@bwindels bwindels merged commit 224ab26 into element-hq:master Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The /join command
3 participants