Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Settings button in Room Context Menu #2692

Merged
merged 4 commits into from
Feb 26, 2019

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Feb 24, 2019

Fixes element-hq/element-web#8781

image

Signed-off-by: Michael Telatynski 7t3chguy@gmail.com

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy t3chguy requested a review from a team February 24, 2019 03:26
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

code looks good to me, but I want to run the feature by @lampholder before merging.

Looks like it navigates the user to the room when they click the setting icon, which is the only questionable part imo.

@lampholder
Copy link
Member

I bounced this past Nad. Broad support for the idea, with just a few tweaks to the UX required before merging:

  • put settings in as the bottom item in the modal, with a line separator above it
  • don't change the room you're viewing when you click settings
  • modify the settings modal title to say 'Room Settings - Room Name' so it's obvious which room's settings you're modifying
    • usual rules apply about what we use for Room Name - whatever we'd put in the room list panel goes in the settings modal title
    • we should not wrap on the settings modal and should truncate with ellipsis

How's that sound?

@t3chguy
Copy link
Member Author

t3chguy commented Feb 25, 2019

How's that sound?

Like a lot more work given that RoomSettings will have to get entirely detached from RoomView so that you can access it from anywhere, including Community View and other such unrelated views

Turns out that work already happened for the redesign 🤷‍♂️
Most of the work is done :D

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy
Copy link
Member Author

t3chguy commented Feb 25, 2019

image

So the changes have proved to be rather simple
image
image

it does feel a bit repetitive to have the Room Name there in the title and also in the input box but I guess that's only on that tab.

…github.com:matrix-org/matrix-react-sdk into t3chguy/room_context_menu_settings
@@ -60,9 +61,12 @@ export default class RoomSettingsDialog extends React.Component {
render() {
const BaseDialog = sdk.getComponent('views.dialogs.BaseDialog');

const room = MatrixClientPeg.get().getRoom(this.props.roomId);
const title = _t("Room Settings") + ' - ' + room.name;
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be _t("Room Settings - %(roomName)s", { roomName: room.name }) because translations

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@turt2live turt2live merged commit 18b27a7 into develop Feb 26, 2019
@t3chguy t3chguy deleted the t3chguy/room_context_menu_settings branch May 25, 2020 18:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants