Skip to content

Conversation

@jimmcgaw
Copy link
Collaborator

@jimmcgaw jimmcgaw commented Mar 6, 2023

No description provided.

@jimmcgaw jimmcgaw requested a review from elimisteve March 6, 2023 05:43
{roomList.map((passphrase) => {
<li className="current-room">
{displayRoomName(passphraseFromUrlHash)}{' '}
</li>
Copy link
Member

Choose a reason for hiding this comment

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

@jimmcgaw Isn't it better to have a consistent room ordering, rather than moving the current room to the top?

Copy link
Member

Choose a reason for hiding this comment

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

Seems easier to navigate, like in Slack or Discord. Yes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Honestly, I don't know.

Copy link
Member

Choose a reason for hiding this comment

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

Okay I'm 90% sure leaving the same room ordering is best. Please make this tweak then let's merge?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@jimmcgaw jimmcgaw force-pushed the cap2_room_switching branch from 75c7162 to d342e6e Compare March 6, 2023 06:30
@jimmcgaw jimmcgaw merged commit f7998d4 into capacitorjs2 Mar 6, 2023
};
const setPassphrase = (passphrase) => {
console.log('setPassphrase() -- TODO: Disconnect the WebSocket, change the current URL hash, auth again, connect via WebSocket');
};
Copy link
Member

Choose a reason for hiding this comment

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

Let's nuke this function but... who cares! Room switching ftw

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.

3 participants