Skip to content

Conversation

@backportbot-nextcloud
Copy link

backport of #2117

Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

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

There is a minor issue with the loading spinner position on checkboxes; I will take care of that.

Until now the model was updated as soon as the checkbox was modified,
which triggered a change event that in turn caused the UI to be updated.
However, if the room could not be successfully set as public or private
nothing was done and the model as well as the UI were kept with the
wrong state (at least, until the model is fetched again). To prevent
this now the model waits for a successful server response to update the
attributes; in case of failure only the checkbox needs to be restored,
as it is the only UI element that changed due to the direct interaction
of the user (in case of success the whole view will be rendered again
due to the room type change, so no need to do anything explicitly in
that case).

As the model is not changed until the successful server response is
received now the checkbox is replaced by a working icon while waiting
for the answer. In a similar way the checkbox is also disabled to
prevent further requests while the previous one is ongoing.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Due to its special nature the password is not stored in an attribute of
the model. Nevertheless, a "changed:password" (always with an empty
value) was triggered as soon as the password was submited. However, as
there is no password attribute that event is not taken into account
anywhere and the UI was not updated until the server response was
received (with further updates once the rooms are fetched again). To
provide feedback to the user that the password is being set now the
confirm button is replaced by a working icon while waiting for the
answer. In a similar way the input is also disabled to prevent further
requests while the previous one is ongoing.

Note that due to the aforementioned behaviour it is not really necessary
to wait for the server response to update the model with the new
password (as it will not be updated anyway), but it is done for
consistency with other requests.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
An empty password can be sent to remove the current password of the
room, which does not match with the "required" semantics. Moreover, the
CSS styles show a red border in invalid input fields, so the red border
was shown too when removing the password despite being a valid
operation.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The input wrapper only contains the text input and the confirm button;
as there will not be more than one confirm button there is no need to
set its rules based on the text input.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Until now the EditableTextLabel was not (visibly) updated until the
server response was received, even if the model was updated as soon as
the new value was submitted (although this updated the hidden label,
which was shown again once the response was received). To provide
feedback to the user that the value is being set now the confirm button
is replaced by a working icon while waiting for the response. In a
similar way the input is also disabled to prevent further requests while
the previous one is ongoing.

Besides that, renaming the room now waits for the server response before
updating the model. This ensures that, in case of error, the label will
still show the old value instead of the new one. An error message is now
also shown to the user in that case.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
@danxuliu danxuliu force-pushed the backport/2117/stable16 branch from d54afc0 to 06fa1c3 Compare August 29, 2019 09:46
Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

@nickvergessen nickvergessen merged commit c8ac396 into stable16 Aug 29, 2019
@delete-merged-branch delete-merged-branch bot deleted the backport/2117/stable16 branch August 29, 2019 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants