Skip to content

Conversation

@danxuliu
Copy link
Member

Instead of immediately updating the UI on some actions (setting the room as public or private, setting the password, and renaming the room) now a loading spinner is shown in the appropriate element until the server sent a successful response; if the action failed then an error message is shown.

This fixes the old behaviour of updating the UI to the new state and then suddenly and silently reverting back to the previous one if the operation happened to fail.

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]>
@nickvergessen
Copy link
Member

Could have also delayed this until the vue rewrite next release...
One release more or less wouldn't have hurt much anymore.

@nextcloud nextcloud deleted a comment from danxuliu Aug 28, 2019
@nickvergessen
Copy link
Member

I removed the backport comment but kept the label.
We will do the backports after the release to keep drone a bit more relaxed

@nickvergessen nickvergessen merged commit fd9837e into master Aug 28, 2019
@delete-merged-branch delete-merged-branch bot deleted the provide-feedback-to-the-user-when-some-operations-are-in-progress branch August 28, 2019 07:23
@nickvergessen
Copy link
Member

/backport to stable16

@backportbot-nextcloud
Copy link

backport to stable16 in #2131

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