Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add to the file manager extension the option to edit a file. #1442

Closed
wants to merge 1 commit into from

Conversation

camilasan
Copy link
Member

@camilasan camilasan commented Sep 25, 2019

The file can be edited in the browser via one of our apps Text,
Collabora or OnlyOffice.

TODO:

  • create the request with the file id and token to be able to open the file in edit more in the browser
    Answer: https://github.com/nextcloud/richdocuments/blob/master/docs/mobile_flow.md
    "Basically it is checking if is supported and if it is. A simple post with the file id gets you the URL to open then there are some messages to handle etc. But I must admit I don't know them all by heart. But @tobiasKaminsky does"
  • "This would also work with OnlyOffice and Text and others? Both require server work, as OnlyOffice currently does not allow to edit files directly. But Julius and me discussed this with OO guys at Conf. For Text Julius is already working on it. It is planned, that this is somehow abstracted in a more generic way, but this is up to Julius."

Also see nextcloud-gmbh/server#14.

edit

@jancborchardt
Copy link
Member

Copying over the specs from the issue:

  • This can replace the "Open in browser" entry as it’s basically the same thing but better
  • "Open in browser" should be called "Open online", just minor wording
  • The entry should be called "Edit collaboratively"
  • For odt/ods/odp documents, we need to check on the server if Collabora Online is installed.
  • For txt/md we need to check for Nextcloud Text on the server

For the last 2 points cc @juliushaertl?

Similarly we could do that for the activity stream in the improved tray menu #877: If a synced file is txt/md/odt/etc the action button could say "Edit" (better wording welcome, but we are short on space).

right click menu
(The mockup has a unified menu for right-click, that’s a separate discussion though so feel free to ignore that. ;)

@juliushaertl
Copy link
Member

juliushaertl commented Oct 28, 2019

The related server API PR for 18 is here: nextcloud/server#17625

The entry should be called "Edit collaboratively"

@jancborchardt We probably should use the exposed apps name that is used for editing (e.g. Edit with Collabora Online) since there might be multiple apps you could open a file with.

@jancborchardt
Copy link
Member

jancborchardt commented Oct 29, 2019

@jancborchardt We probably should use the exposed apps name that is used for editing (e.g. Edit with Collabora Online) since there might be multiple apps you could open a file with.

I wouldn’t expose the app name as this is not really relevant to regular people, it’s more confusing. :) It’s focused on Collabora Online, OnlyOffice and Text, and if there are multiple apps to edit with it should just pick 1 for simplicity.

@tobiasKaminsky

This comment has been minimized.

@juliushaertl
Copy link
Member

Then we could just use "Edit collaboratively" if only one editor is available, and show the names otherwise I'd say.

The file can be edited in the browser via one of our apps Text,
Collabora or OnlyOffice.

Signed-off-by: Camila San <hello@camila.codes>
QMimeDatabase db;
QMimeType type = db.mimeTypeForFile(fileData.localPath);
if (capabilities.hasRichDocuments() && capabilities.supportedRichDocumentsMimetypes().contains(type.name().toLatin1())){
listener->sendMessage(QLatin1String("MENU_ITEM:EDIT") + flagString + tr("Edit via ") + capabilities.richDocumentsProductName());
Copy link
Member

@jancborchardt jancborchardt Dec 10, 2019

Choose a reason for hiding this comment

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

Just to clear this up, we only want it to say "Edit", right? No product name, no "… collaboratively". Can you confirm @karlitschek?

cc @camilasan fyi

Copy link
Member

Choose a reason for hiding this comment

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

(Also, "Edit" should be the first in the list, before the Sharing entries – because you can also do that when the document is not shared.)

Copy link
Member

Choose a reason for hiding this comment

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

@jancborchardt Yes. I would only say 'Edit'. The app that is used depends on the mime type and is not really important for the user.

Copy link
Member

@jancborchardt jancborchardt Dec 10, 2019

Choose a reason for hiding this comment

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

@karlitschek thanks, agreed!

Suggested change
listener->sendMessage(QLatin1String("MENU_ITEM:EDIT") + flagString + tr("Edit via ") + capabilities.richDocumentsProductName());
listener->sendMessage(QLatin1String("MENU_ITEM:EDIT") + flagString + tr("Edit"));

Copy link
Member

Choose a reason for hiding this comment

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

While implementing this feature with OO, we found a small "problem": you can open also pdf files, which are not editable.
So we thought about "open (remotely)" instead of "edit" as this is more generic.

Copy link
Member

Choose a reason for hiding this comment

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

@tobiasKaminsky can’t we just exclude PDF from it instead? Then the entry will simply say "Open in browser" as specified at #1442 (comment)

That’s much better than complicating the wording for the regular case.

@jancborchardt
Copy link
Member

So this can be closed as it’s superseded by #1735, right? :) @misch7 @camilasan

@jancborchardt
Copy link
Member

This was fixed by #1735, closing this hence. :)

@jancborchardt jancborchardt deleted the feature/context-menu-edit branch March 24, 2020 10:16
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.

5 participants