Say the room was not found when rejoining one you have left - #34639
Say the room was not found when rejoining one you have left#34639hayaksi1 wants to merge 1 commit into
Conversation
github-actions
Bot
added
the
Z-Community-PR
|
Please provide a screenshot |
Visual Changes
|
americanrefugee
left a comment
americanrefugee
left a comment
There was a problem hiding this comment.
Looking good, but I'd like to make some changes:
- Use a smaller modal
- Update the text to be more human readable
- Change the description text color to
Secondary
Here is a design proposal:
10c7d39 to
bccb3c7
Compare
Visual Changes
|
|
The PRs don't seem to be aimed at altering the dialogue in terms of readability. Rather it addresses an issue that happens to have a visual component. |
|
"Can't" and "Can" can be confused, particularly when scanning or hearing the text. Here I used "could not" since it implies an outcome or result of the attempted search and join request.
Alternative1.
"The" seems more appropriate since it keeps the context of the last action, reporting a result rather than pointing to a subject. 2.*(Prefer this one)
Putting the reason summary as the heading may work well too as the error implies a failure to join. |
I like prefer this one as well. Good call @TemoCrevlis |
Pressing rejoin on a room you had left and getting a 404 back produced the error written for somebody who pasted a bare room ID: that room IDs are internal identifiers, that a list of servers was needed, and that an address should be used instead. Nobody on that path typed anything - the preview bar offering the rejoin is only shown because the client knows the room and knows the membership is leave, so the advice cannot apply. A 404 for a room the client already knows and has left now gets its own message, hedged rather than declaring the room deleted, because a 404 here can equally mean that no server your homeserver can reach is still in it. The bare room ID message is unchanged and still shown for the case it was written for. Not being found is the whole of what happened, so it reads as the heading and the body is left to carry only why it might have happened. That costs this one branch a title of its own where the other join failures share one, but the alternative spends the most-read line restating the failure above a sentence that already explains it. The shared title still gains the noun it was missing, "Failed to join room", for the failures that keep it. The dialog carried the shared 60vw width, which stranded one short sentence across 784px with a 68px hole above the button. It is now sized to its message, the description takes the secondary text colour, and the single button spans the width it belongs to. A wrapper class scopes that to the join failure dialog rather than changing the error dialog generally, since the others have not been looked at.
bccb3c7 to
add1136
Compare
Visual Changes
|
hayaksi1
changed the title
|
@t3chguy @americanrefugee Can be merged? |
|
The visual changes LGTM, but I'll let @t3chguy respond to whether or not this can be merged. |
|
@americanrefugee can you approve the PR then, you still have 
|
6 participants
Pressing rejoin on a room you had left and getting a 404 back produced the error written for
somebody who pasted a bare room ID: that room IDs are internal identifiers, that a list of servers
was needed, and that an address should be used instead. Nobody on that path typed anything. The
preview bar offering the rejoin is only shown because the client knows the room and knows the
membership is leave, so the advice cannot apply.
A 404 for a room the client already knows and has left now gets its own message saying the room
could not be found. The wording is deliberately hedged rather than declaring the room deleted,
because a 404 here can equally mean that no server the homeserver can reach is still in it.
The bare room ID message is unchanged and still shown for the case it was written for.
Tests: a case in
RoomViewStore-test.tsfor a 404 on a room whose membership is leave, assertingthe description; it fails without the change.
Fixes #25627
Checklist
public/exportedsymbols have accurate TSDoc documentation.