nextcloud / files_lock

GNU Affero General Public License v3.0
24 stars 8 forks source link

Change the lock icon when automatic file locking is only a warning that editing is in progress #157

Closed Jerome-Herbinet closed 3 months ago

Jerome-Herbinet commented 1 year ago

I did a lot of testing of the automatic (smart) and manual locking functionality in Nextcloud Files because I didn't understand all the nuances of how it worked.

When someone opens an ODT file that has been shared with them, with Nextcloud Office (browser), the file in question is marked with a padlock :

@jancborchardt @szaimen @ChristophWurst ; what do you think ?

See also #156 issue (related topic)

How to use GitHub

ChristophWurst commented 1 year ago

I'm not familiar enough to help with this code

jancborchardt commented 1 year ago

cc @juliushaertl from the Office team who would know this or knows who knows more about it. ;)

juliushaertl commented 1 year ago

I did a lot of testing of the automatic (smart) and manual locking functionality in Nextcloud Files because I didn't understand all the nuances of how it worked.

I guess we could definitely improve documentation around that.

When someone opens an ODT file that has been shared with them, with Nextcloud Office (browser), the file in question is marked with a padlock :

* Users using the web interface via their browser can, despite the padlock, can continue to edit the file (collaborative editing by several users). In this case, the padlock is a source of confusion, especially as the file's context menu also includes "Locked by ..." (even more confusing). So, in this particular case, I think you should not display a padlock, but a dedicated person icon that may look like this [pngfreepic.com/wp-content/uploads/2021/03/User-Edit-Icon-pngfreepic-3.png](https://pngfreepic.com/wp-content/uploads/2021/03/User-Edit-Icon-pngfreepic-3.png). Also, in the context menu, "Locked by ..." should be replaced by "Edited by ...".

People that are actively working on a document through collaborative editing (text, collabora) have an automatic shared lock on the file to avoid other editors or local clients changing it in the meantime. I think showing the actual users is a nice idea. For the wording I'm not fully sure, but that is something for @nextcloud/designers to maybe evaluate further.

In regards of showing the icon/avatars this is something were we are currently limited by the files API to inject actions: https://github.com/nextcloud/files_lock/issues/154

* Users using their operating system's file manager (+ desktop synchronization client) actually have their file locked (a padlock does appear under OSX and GNU/Linux, but not under Windows, but you're not responsible for this, as it's due to Windows UI, please confirm). In this case, the padlock, when it appears, indicates a lock and there's no possible confusion, apart from the fact that no padlock appears under Windows.

The desktop client (or mobile clients) should be aware of the locked state through the regular PROPFIND discovery of files and then indicate that. If that doesn't work on windows this would be something for a bug report in https://github.com/nextcloud/desktop

Jerome-Herbinet commented 1 year ago

@juliushaertl, thanks for your answer.

About wording, I've noticed that when a file is locked automatically (smart locking), wording is : "Locked by {name of app}". In the other hand, when the file is manually locked, wording is "Locked by {user display name}".

In both case, it uses the following wording found in Transifex : "Locked by {0}"

In the program, can't we make a condition like :

If "0" is an app's name, wording is "Currently edited with {name of app}", else if "0" is a user's display name, wording becomes "Currently edited with {user display name}".

I mean, if there are limitations upstream, can't we analyze downstream what "0" contains in order to render different wording + icon, depending on the case ?

juliushaertl commented 1 year ago

Sure, differentiating is definitely a good idea. From my perspective lock should still be a word being used there as it has clear implications on that you might not be able to edit/upload then, but I'd also be open for other suggestions, especially considering that space is limited in the three dots menu.

In terms of clearer wording I thought about something like this, but might be too long:

There is actually a 3rd case which is WebDAV locking if you are using a WebDAV mount on windows for example. In this case we currently also show "Locked by {user}". Most meaningful label would be this but sounds too technical:

Jerome-Herbinet commented 1 year ago

Sure, differentiating is definitely a good idea. From my perspective lock should still be a word being used there as it has clear implications on that you might not be able to edit/upload then, but I'd also be open for other suggestions, especially considering that space is limited in the three dots menu.

In terms of clearer wording I thought about something like this, but might be too long:

* Automatically locked by collaborative editing with {name of the app}

* Manually locked by {user display name}

There is actually a 3rd case which is WebDAV locking if you are using a WebDAV mount on windows for example. In this case we currently also show "Locked by {user}". Most meaningful label would be this but sounds too technical:

* Automatically locked through WebDAV by {user}

@juliushaertl, I agree with what you say except one thing.

If I put myself in the place of an average user, the "locked" word means that the file is locked (read-only) ... and I really think that a lot of users will believe that they can't edit the document as long as they see "locked".

If "Locked" is used despite it's not really the case, in terms of ergonomics, this compromises the collaborative spirit of the tool.

That's why I think that wording should be (in case of automatic locking when file has been open from the web UI) :

I think it would be interesting to get some other opinions ...

juliushaertl commented 1 year ago

Collaborative editing underway with {name of the app}

Yeah makes sense, maybe something to also differentiate the message between web and clients, to clearly indicate on the client side that you cannot edit because it is currently being edited collaboratively.

Jerome-Herbinet commented 1 year ago

Collaborative editing underway with {name of the app}

Yeah makes sense, maybe something to also differentiate the message between web and clients, to clearly indicate on the client side that you cannot edit because it is currently being edited collaboratively.

It makes sense.

In concrete terms, how do we move forward? Is it worth mentioning other people to make progress on this particular subject? It would be a great step forward for the locking system 😉

juliushaertl commented 1 year ago

I pinged designers on the chat to gather some input, let's wait for that :)

jancborchardt commented 1 year ago

I think the text is not even so much a problem as it's not so visible – and we shouldn't make it too complex. The main issue I notice often and stumble over is the icon being the lock when someone is editing it only in the web interface. If we rectify that, then I think the majority of confusion will already be solved.

So let's start with that and reevaluate after?

juliushaertl commented 1 year ago

So what would you rectify there? The file should still be locked to prevent desktop/mobile clients from editing, right? Then we just don't show the file as locked in the web UI in case it is locked due to collaborative editing (when the default file action click would bring you to that editing session)? Other clients still show the lock status?

jancborchardt commented 10 months ago

@juliushaertl yeah, I would say so. Cause it's basically not really locked for the web interface as you can totally collaborate. If there is no restriction, we don't need to make it look like there is (at least not on that platform).

Maybe even not show it as locked for mobile either since we use the the embedded Text app there too?

Jerome-Herbinet commented 3 months ago

Fixed a few months ago.