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

Add leave room warning for last admin #9452

Merged
merged 8 commits into from
Mar 22, 2024

Conversation

Arnei
Copy link
Contributor

@Arnei Arnei commented Oct 19, 2022

If the last room administrator leaves a room, other users cannot gain admin privilges anymore, leaving the room in an unmoderable state. To help in avoiding this scenario without actually preventing an admin from leaving the room if they really want, this commit adds a new warning message.

Attempts to help with: element-hq/element-web#2855

Screenshots as recommended by robintown

With no warnings:
Screenshot 2024-03-21 at 11 53 35

With warnings:
Screenshot 2024-03-21 at 11 53 49

Signed-off-by: Arne Wilken arnepokemon@yahoo.de

type: enhancement


Here's what your changelog entry will look like:

✨ Features

  • Add leave room warning for last admin (#9452). Contributed by @Arnei.

If the last room administrator leaves a room, other users cannot
gain admin privilges anymore, leaving the room in an unmoderable
state. To help in avoiding this scenario without actually preventing
an admin from leaving the room if they really want, this commit
adds a new warning message.

Attempts to help with: element-hq/element-web#2855

Signed-off-by: Arne Wilken arnepokemon@yahoo.de
@Arnei Arnei requested a review from a team as a code owner October 19, 2022 10:53
@github-actions github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Oct 19, 2022
@github-actions github-actions bot added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Oct 19, 2022
@t3chguy
Copy link
Member

t3chguy commented Oct 20, 2022

Hey @Arnei thanks for your contribution, any chance of satisfying the code coverage test requirement?

@Arnei
Copy link
Contributor Author

Arnei commented Oct 21, 2022

Hi @t3chguy, I could not find a related test suite for this. Could you point me to it or to another test suite that would be a good example for this?

@t3chguy
Copy link
Member

t3chguy commented Oct 21, 2022

Actually I take it back, MatrixChat is currently too indebted to be tested, you can skip it for now. The web app team have plans to fix the situation. Just please resolve merge conflicts.

Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

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

This will need a signoff from the design team - could you please add a screenshot of the warning to make it easier for them to review the copy?

src/components/structures/MatrixChat.tsx Outdated Show resolved Hide resolved
src/components/structures/MatrixChat.tsx Outdated Show resolved Hide resolved
@robintown robintown requested review from a team and removed request for germain-gg and dbkr October 21, 2022 15:23
getContent already does the || {} step

Co-authored-by: Robin <robin@robin.town>
Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

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

The code looks good, just waiting on design now.

@Arnei
Copy link
Contributor Author

Arnei commented Dec 13, 2022

Would it be rude to ping Design for their opinion?

<span className="warning" key="last_admin_warning">
{ ' '/* Whitespace, otherwise the sentences get smashed together */ }
{ _t("You are the sole person with the highest role in this room. " +
"If you leave, the room could become unmoderable. Consider giving " +
Copy link
Contributor

Choose a reason for hiding this comment

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

unmoderable doesn't feel like a word. unmoderatable sounds better to me but I also don't see it as a real word so probably need to rearrange the prose to accommodate proper English. Perhaps:

"If you leave, the room might not be able to be moderated anymore."

Switches out a made-up word for a real phrase.
@daniellekirkwood
Copy link

daniellekirkwood commented Mar 29, 2023

Discussed the content with @americanrefugee and here's our suggestion...


Headline:
Sure you want to leave?

Body copy:
You’re the only <administrator/moderator> in this room. If you leave, nobody will be able to change room settings or take other important actions.

Consider making someone else <an administrator/ a moderator> before you go.


We'd use this copy for both cases as an admin or moderator leaving a private room should know that it's private and should know the circumstances needed for re-entry.

It would also be great if we could make that button red instead of green :)

@aaronraimist
Copy link
Contributor

@daniellekirkwood I think you probably meant to @ the Aaron that works at Element :)

@daniellekirkwood
Copy link

@daniellekirkwood I think you probably meant to @ the Aaron that works at Element :)

I'm so sorry! Yes I did :)

@dbkr dbkr removed request for a team March 21, 2024 11:55
@dbkr
Copy link
Member

dbkr commented Mar 21, 2024

I've resolved the conflicts and incorporated the copy from the feedback, with separate text for the admin / mod case. I left the title as-is because I think this might need splitting out from the general "leave room" action, and not sure if it also wants to apply to the case when there aren't warnings?

The type utils file got deleted and we just don't have these kind of type checks in react-sdk (for better or worse) so I've just inlined it.

Also put it in strong tags for consistency with the other warnings. Screenshots updated. Probably wants eyes from someone else to check my work.

@richvdh
Copy link
Member

richvdh commented Mar 22, 2024

@dbkr can you update the screenshots?

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

seems plausible.

The coverage gate is, of course, complaining, but I won't insist on it being fixed.

@dbkr
Copy link
Member

dbkr commented Mar 22, 2024

Screenshots should be updated? And yeah, seems like we'd already decided it was okay without the coverage tests.

@dbkr dbkr merged commit 03dc48b into matrix-org:develop Mar 22, 2024
22 of 23 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants