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

Enhancements to room power level settings #1440

Merged
merged 8 commits into from
Oct 11, 2017

Conversation

turt2live
Copy link
Member

Changes:

image

Signed-off-by: Travis Ralston <travpc@gmail.com>
This does mean that the strings will need to be re-translated, but now they may be more accurate because the comma is not assumed in the code.

Signed-off-by: Travis Ralston <travpc@gmail.com>
Signed-off-by: Travis Ralston <travpc@gmail.com>
Signed-off-by: Travis Ralston <travpc@gmail.com>
Signed-off-by: Travis Ralston <travpc@gmail.com>
Fixes element-hq/element-web#4866

Signed-off-by: Travis Ralston <travpc@gmail.com>
@pafcu
Copy link
Contributor

pafcu commented Oct 4, 2017

Would it be possible to do some grouping and maybe change the order a bit? E.g. kicking and removing messages is clearly about moderation, while setting the topic and room name is about room configuration. You also have "To change the room's ..." or similar in each case except for the topic.

Also, how does the "To configure the room" setting affect the other room configuration things (like setting avatar, topic, etc.) ?

@turt2live
Copy link
Member Author

Sections would be a great addition, although I don't know how they'd look in the settings page.

The configure room permission is unchanged by this. It is the default for who can set events that aren't defined in the list.

@pafcu
Copy link
Contributor

pafcu commented Oct 4, 2017

It is the default for who can set events that aren't defined in the list.

That should probably be explained somewhere. Or perhaps change the text to "To configure other room settings ..." ?

@@ -35,6 +35,31 @@ function parseIntWithDefault(val, def) {
return isNaN(res) ? def : res;
}

const plEventsToLabels = {
// These will be translated for us later.
// TODO: _td() these when https://github.com/matrix-org/matrix-react-sdk/pull/1421 lands
Copy link
Contributor

Choose a reason for hiding this comment

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

This has landed, so feel free to _td these

@@ -364,6 +389,11 @@ module.exports = React.createClass({
var powerLevels = this.props.room.currentState.getStateEvents('m.room.power_levels', '');
powerLevels = powerLevels ? powerLevels.getContent() : {};

for (let key of Object.keys(this.refs).filter(k => k.startsWith("event_levels_"))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that this would require a lot more work if we used state, this is probably fine. @dbkr do you have thoughts on this?

Copy link
Member Author

Choose a reason for hiding this comment

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

tbh I only used refs because the rest of the powerlevels use refs.

@lukebarnard1
Copy link
Contributor

FTR there is a plan to totally reorganise room settings so that there are sections and a uniform concept of "saving".

Separately, I mocked up some designs for a redesigned power level display:
https://docs.google.com/document/d/1apqMz9rJUQgab5ZwcFfJFLBbLpHtjClK8i_qC0KmqR8/edit

@turt2live
Copy link
Member Author

turt2live commented Oct 11, 2017

@lukebarnard1 I've added _td to the strings and merged masterdevelop in.

The RoomSettings (and UserSettings) are a bit of a mess in my opinion. I'm okay with the current power level structure in the settings personally, it's fairly clear who can do what. Although I do like your proposed idea of showing who can do what closer to the actual permissions, instead of having to figure it out yourself (design C, mostly).

@lukebarnard1 lukebarnard1 merged commit d2b9fcc into matrix-org:develop Oct 11, 2017
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