CollaboraOnline / online

Collabora Online is a collaborative online office suite based on LibreOffice technology. This is also the source for the Collabora Office apps for iOS and Android.
https://collaboraonline.com
Other
1.83k stars 697 forks source link

Popup window text confusing when editing same document #9915

Open xmarcux opened 1 month ago

xmarcux commented 1 month ago

Hi all, When a user is editing a document and a second user opens the same document, you get a popup with a warning.

image

This is the view for the user opening the document while somebody else is already editing the document. Sometimes the user that first opened and edited the document get the same popup at the same time. Guessing that it has to do with sessions...

This text is quite confusing and our users get unsure what to do. “I have just opened the document and have not done any changes, what is this?”

After some testing we realize that this must be due to some type of caching on the server side. Would it be possible to change the wording or behavior of this functionality? One way would be to change the wording to something like:

Other users are editing this document. Multiple sessions may overwrite some data.
Do you want to proceed?
Button: "No" "Yes"

Or just inform the user and do not warn at all:

Other users are editing this document.
Do you want to proceed?
Button: "No" "Yes"

The behavior becomes the same as pressing “Overwrite” button in both suggestions above. The "Discard" button is kind of dangerous as if the second user opening the document click "Discard" all changes that the first user have changed will disappear.

The wanted behavior is the same as clicking "Overwrite" for the second user opening the document, changes that the first user have made in the document is still there after the second user has loaded the document.

Thanks for a great system!!

pedropintosilva commented 1 month ago

Thanks @xmarcux for the report and insight.

I think there is room for improvement. I have added a code pointer bellow but before that I think it's good to clarify the purpose of this dialog before someone starts to work on that: That dialog servers the purpose of avoiding document conflicts. Example: User A was editing it online in the web browser while user B has edited a previous version locally and that has already arrived to the storage server via integrators sync client, another quit common case is when multiple users were editing at the same time (possibly with different internet connections) the document until they have reach a conflict. There are other examples but generally speaking that's the main idea. For that we need to offer the user to either discard his version of the document or overwrite the version that is already in storage.

Dialog:

Here is what those changes would look like:

image

Here is the code pointer: showDocumentConflictPopUp

```js _showDocumentConflictPopUp: function() { var buttonList = []; var callbackList = []; buttonList.push({ id: 'cancel-conflict-popup', text: _('Cancel') }); callbackList.push({ id: 'cancel-conflict-popup', func_: null }); buttonList.push({ id: 'discard-button', text: _('Discard') }); buttonList.push({ id: 'overwrite-button', text: _('Overwrite') }); callbackList.push({id: 'discard-button', func_: function() { this.sendMessage('closedocument'); }.bind(this) }); callbackList.push({id: 'overwrite-button', func_: function() { this.sendMessage('savetostorage force=1'); }.bind(this) }); if (!this._map['wopi'].UserCanNotWriteRelative) { buttonList.push({ id: 'save-to-new-file', text: _('Save to new file') }); callbackList.push({ id: 'save-to-new-file', func_: function() { var filename = this._map['wopi'].BaseFileName; if (filename) { filename = L.LOUtil.generateNewFileName(filename, '_new'); this._map.saveAs(filename); } }.bind(this)}); } var title = _('Document has been changed'); var message = _('Document has been changed in storage. What would you like to do with your unsaved changes?'); this._map.uiManager.showModalWithCustomButtons('document-conflict-popup', title, message, false, buttonList, callbackList); } ```
Darshan-upadhyay1110 commented 1 month ago

Cancel: @Darshan-upadhyay1110 can you check if this does the same as the 'dialog' close (x)`? If so I would propose it to remove it (similar as the @xmarcux suggested)

I have just tested this and @xmarcux you are right this is duplication of functionality here. We can remove cancel button from this Dialog.

CC: @pedropintosilva