[UX] Allow uploading a file from the link dialog in rich-text editor

jenlampton commented 8 years ago

When reviewing the Rich-text editor someone mentioned

I've grown used to pressing link and then following the options to upload a file.

It might be worthwhile considering adding a file-upload tool to the link option in Backdrop as well.

What do others think, would this be a useful feature for 80% of sites out there, or is this better left to contrib?

~~Initial PR now available for review https://github.com/backdrop/backdrop/pull/2082~~ Improved PR (also from Graham-72) now available for review https://github.com/backdrop/backdrop/pull/2103

Graham-72 commented 8 years ago

pressing link and then following the options to upload a file

I definitely prefer to work this way, both for uploading images and for making hyperlinks to documents. If it were to be in contrib would it appear to the user as part of CKeditor? That seems important to me for the sake of ease-of-use by clients.

olafgrabienski commented 8 years ago

Some late feedback: I would not expect to be able to upload a file pressing "Link" or a link symbol. How do other systems work? When I remember correct, WordPress for instance has a "Add files" link at the top of the editor.

quicksketch commented 7 years ago

This is a duplicate of an older issue @Graham-72 filed: #1586. Let's consolidate there.

quicksketch commented 7 years ago

Oh sorry this request is different. It's about uploading a file from the link dialog, not being able to link to a file. Reopening...

jackaponte commented 7 years ago

+1 to this being a valuable/needed addition to the rich-text editor! (See #2619 which I've closed as a duplicate of this issue.)

Graham-72 commented 7 years ago

+1 from me too.

Al-Rozhkov commented 7 years ago

I have few questions.

Should we provide autocomplete for exisiting files? Should we update file usage information when link-to-file is inserted?

WordPress for instance has a "Add files" link at the top of the editor.

Yes, it is. Isn't it better to provide single button for Media browser (since we plan to include it in core)? I doubt that file managment is obvious function for "Link" button.

jenlampton commented 7 years ago

Isn't it better to provide single button for Media browser (since we plan to include it in core)? I doubt that file management is obvious function for "Link" button.

I agree with this. I think it would make more sense to have a link option from the file/image dialog, than to have an file/image option on the link dialogue.

Graham-72 commented 6 years ago

I would very much like to see this in core. It would meet an important user need. I envisage that the link to a file procedure might now look like this. Is this about right? screenshot-www oliverweb net-2018-01-03-07-27-15-393

Unfortunately I do not have the javascript expertise needed to revise the backdroplink plugin in the ckeditor module, though I think the necessary functions must be there already, or in the backdropimage plugin. Can anyone help with this please?

Graham-72 commented 6 years ago

I have now found a solution that does not require any change to the javascript plugins. With my initial attempt at a PR the options look like this: screenshot-2018-1-27 edit page test page cgwebdesign 1

herbdool commented 6 years ago

Hi Graham, good work on the PR. I haven't tried it but looks like you've made progress. I think you need to rework this, however, to account for the UI issue brought up by Jen and Al. Putting it on the link button is not obvious, instead the should be a link option on there file/image button

Graham-72 commented 6 years ago

@herbdool thanks for your comment. I am in two minds about this and perhaps we need to clarify how the upcoming media solution(s) will fit with all this.

My thinking at the moment, based on a clear user need, is that my users need to be able to create hyperlinks out of text in wysiwyg. Their hyperlinks may link to another page on the site, or a URL on a different site, or a document they want to make available by uploading to their site e.g. a pdf file.

This to my mind makes 'link' the more natural step in their workflow. I'm very open to other suggestions but my users need an easy method now, not jam tomorrow 😄 .

herbdool commented 6 years ago

Then maybe you're better off making a contrib module. Unless you can convince people this needs to be in core.

Graham-72 commented 6 years ago

Yes, I think it is a serious omission from Backdrop core, mainly because my users expect their WYSIWYG to be able to do this.

opi commented 6 years ago

Nice idea @Graham-72, Meanwhile, what about the "insert" drupal module ?

Graham-72 commented 6 years ago

@opi yes I have tried the Insert module, which is available for Backdrop. It enables one to move a link to a file into the WYSIWYG area but the visible hyperlink is the file name. My users expect to be able to link from any piece of text they choose. I can do this in the source code of course, but I cannot expect them to do this.

olafgrabienski commented 6 years ago

Still not sure what makes more sense: a link option from a file dialog or a file option on the link dialog. (At the moment we don't have a file dialog in the editor, there is only the image dialog.)

I was however curious about the PR and had a look at the sandbox site:

Uploading a file through the link dialog works.
I found it confusing that the placeholder text ( files/uploads/myfile.pdf) in the URL tab of the link dialog is file related.
I was asking myself if the help text of the URL tab (Enter a known path, or start typing to get suggestions) means that I would even be able link to a former uploaded file. Didn't manage to do so though.

My conclusion: functionality is good, user interface needs work

laryn commented 6 years ago

I think there is definitely overlap between the options but also some difference. Is it a bad idea to consider having the ability to upload in two places? Although I agree with the concept that they both link to the same future "Media manager" interface in some way.

Example use case where the image button makes sense to include a link option:

You are inserting an image into the editor and want to link it to something else

Example use case where the link button makes sense to include an upload option:

You are highlighting some text in the editor and want to link it to a PDF file

UI comments from @Al-Rozhkov (below, which would be part of the Media Manager solution most likely) and @olafgrabienski (in the comment above) are on point:

Should we provide autocomplete for exisiting files? Should we update file usage information when link-to-file is inserted?

olafgrabienski commented 6 years ago

@laryn I agree, uploading in more than one places isn't a bad idea.

Another question regarding file upload in the link dialog: does it make sense to restrict the allowed file types, and where would one configure that?

herbdool commented 6 years ago

I took a look around at other rich text editors and I didn't see any that reused the link button to also be a file upload button. Not only does the approach in this PR seem to buck the trend of other editors, it is also unintuitive in my mind. Why is there a file placeholder text for something that's supposed to be for any kind of link? It's trying to merge two different kinds of dialogues into one.

File uploading is different enough that it needs to be treated as its own process and not take over the link dialogue. Take a look at Github editor for example, it has a separate file upload interface below the text body. Wordpress has a separate button for adding media.

olafgrabienski commented 6 years ago

@herbdool I see your point! I've also had a look at WordPress, and even there it doesn't seem to be possible to upload (or select) a file using the link dialog. And I agree that adding a file related UI to the link dialog might be confusing.

However, so far the alternative suggestions in this thread don't meet the use case described by @Graham-72 (users are able to link a file from any piece of text without the need to touch the source code):

In the Github editor you have to change the (Markdown) source code to change the link text from file name to something else. (Probably fine for most Github users though.)
IIRC, in WordPress you have to change the file description in the media library to achieve the same. Quite complicated.
A link option on the Backdrop / CKEditor image button would be nice - but restricted to images.

At the moment, I've no idea how to accomplish the feature request otherwise, that's why I find the approach of @Graham-72 still interesting, given that the UI gets improved and there will be answers to open questions.

@Graham-72 Are you also interested in workarounds? I've had similar requests from editors of Drupal and Backdrop sites. I suggest them to use a file field and to just mention the files in the text, something like "Lorem ipsum call for papers (see file in the download section at the bottom of this article)." So far, most of them found the suggestion reasonable. Further pros: well organized posts, and restricting file types is easy.

Graham-72 commented 6 years ago

I'm afraid I do not understand why it is so objectionable to provide this from the link button. In D7 with CKeditor and IMCE my users start from the link button and then have several more steps to complete their hyperlink to a file they upload. I see it as a 'natural' workflow.

herbdool commented 6 years ago

I believe part of the differences lie in those who have used IMCE and those, like myself, who don't use it but rather use the Media module in D7. Or those people using other systems like WP. I believe it's a fairly peculiar to IMCE and I'm not comfortable replicating it. I don't see it as "natural".

A separate upload button makes more sense.

docwilmot commented 6 years ago

Actually CKEditor does in fact have the file upload UI in the link dialog instead of the image/media dialog.

capture

herbdool commented 6 years ago

Fair enough @docwilmot -- looks like you're trying out a different version of CKEditor. Is that version 4? I was looking at the latest version 5, which doesn't have the upload functionality as part of the link. It's not clear to me if they're still coming up a UI for file uploads, and if they're going to stick with the same multi-tabbed dialogue or have a separate button.

ckeditor5

With CKEditor 5 they talk about simplifying the toolbar (as you can see with the photo above) so I suspect that they're more aware that the editor shouldn't be too complicated so that it can work better in mobile devices.

@olafgrabienski I'm not sure if I understand what you mean by "in WordPress you have to change the file description in the media library to achieve the same. Quite complicated." I tried it now and it's all point and click--pretty easy. Click the Add Media button, select file and then select option to insert into text.

Graham-72 commented 6 years ago

Yes, it would be good if the autocomplete function for Insert URL were to include previously uploaded files as well as nodes, users, taxonomies and views. I imagine this can be done from the data in the database table ‘file_managed’ and I will attempt this.

Admin control of the file types that can be uploaded would seem useful. Perhaps the best place for this would be a new section ‘File Uploading’ in Configuration > Content authoring > text editors and formats, similar to the existing section ‘Image Uploading’.

I think there is no harm, perhaps benefits, in having two routes for uploading and linking files, provided the appearance and wording is clear and identical where appropriate. Maybe the existing wording initially proposed here needs changing, for example I think the Wordpress ‘Link to existing content’ is more helpful. Suppose we have the following for our Insert Link options screenshot-2018-1-30 edit page test page cgwebdesign

olafgrabienski commented 6 years ago

I'm not sure if I understand what you mean by "in WordPress you have to change the file description in the media library (...)"

@herbdool Sorry, it wasn't the file description but the title. Let me describe, in WordPress, to link a file from any piece of text, you do the following:

Select the text you'd like to put the link on, let's say "Hello world".
Hit the "Add media" button above the text editor. You see the Media library.
Choose a file, or upload a file, e.g. dummy.pdf.
Hit the "Insert into page" button.

As a result, the link has been created, which is fine. But the originally selected text "Hello world" has been replaced by the automatically created file title, in this case "dummy". To avoid the replacement in the first place, you have to edit the file title in the media library, but who knows that? Even more interesting: to fix an unwanted link text replacement, you can use the link dialog instead of the media library, so for linked files you end with a mixed use of media library and link dialog.

olafgrabienski commented 6 years ago

Admin control of the file types that can be uploaded would seem useful. Perhaps the best place for this would be a new section ‘File Uploading’ in Configuration > Content authoring > text editors and formats, similar to the existing section ‘Image Uploading’.

Good place, imho.

Maybe the existing wording initially proposed here needs changing, for example I think the Wordpress ‘Link to existing content’ is more helpful.

@Graham-72 WordPress has two input areas in the link dialog: Enter the destination URL (URL plus Link text) and Or link to existing content (Autocomplete). The link dialog in Backdrop / CKEditor has a combination of URL and autocomplete instead, that's why "Link to existing content" isn't sufficient, "URL" should also be mentioned, maybe "URL or existing content".

Enter the destination URL

Graham-72 commented 6 years ago

@olafgrabienski I think that 'Link to existing content' in its full sense includes entering a URL but maybe this should be spelt out in the help text. As I see it, the user has to choose either to link to an existing item (node, file or whatever) or upload new content with the 'upload and link to a file' option. Hopefully by including files in the autocomplete function it will only be external URLs that will need to be entered manually. Perhaps the help text could be: Enter any known URL or start typing to get suggestions of existing content in this site.

Though personally I would prefer not even to mention 'URL' to my non-technical users.

Graham-72 commented 6 years ago

Thought: Are we targeting non-technical users? Sometimes I doubt it. ❓

quicksketch commented 6 years ago

The discussion here is great. I don't want my opinion to count any more than anyone else's here, I don't really know what the best solution is either. However, I think the approach being taken so far is sensible and adds a much-needed capability to Backdrop: upload a file and link to it from within the editor. As with the image uploading, it would need to be configured to enable it for each text format, so if it's not right for your site you would be able to disable it.

jenlampton commented 6 years ago

Yes, thanks everyone for continuing this discussion :)

I think the reason we don't all agree on which workflow is more intuitive, is that each solution is intuitive to different kinds of thinkers. I personally would never think to click on a LINK button to upload a FILE, however, I also understand that there are people who would!

This is a different scenario than uploading an image because when you upload an IMAGE, an IMAGE tag is inserted into the editor. After uploading a FILE, a LINK tag should be inserted into the editor.

If we could also give people a FILE button (or eventually, a MEDIA button) that allows a file upload and then embeds a link, that tool would solve the user-experience issue for thinkers like me.

But if adding an upload option to the link dialog solves the user-experience issue for thinkers like @Graham-72's users, why not put in both tools, like @laryn suggests?

(I did initially have some concerns about CKEditor removing the file upload option from the link dialog that was in version 4 - fearing that they might have learned that it was a non-ideal workflow for uploading files - but after reaching the conclusion about our 2 kinds of thinkers, I've since gotten over those concerns.)

Graham-72 commented 6 years ago

I think it will be great to provide for both types of thinkers. Meanwhile, for the 'linkers', progress on this issue seems good. My PR for #2963 is working well in my test site and the facility is a great help. Also, with the PR for #2964 we will have admin control for each text format as mentioned by @quicksketch .

My final hope is to add the ability to edit the link text as is possible in Wordpress. This would all then be awesome IMHO.

jenlampton commented 6 years ago

This would all then be awesome

Graham-72 commented 6 years ago

After further work I have now provided a combined PR at https://github.com/backdrop/backdrop/pull/2103 that includes and updates my PRs https://github.com/backdrop/backdrop/pull/2082, https://github.com/backdrop/backdrop-issues/issues/2963 and https://github.com/backdrop/backdrop-issues/issues/2964.

Graham-72 commented 6 years ago

My PR is now, I think, complete and functional except for the future capability to edit the link text in WYSIWYG for which I need help from @quicksketch to modify the javascript. Thank you @herbdool for reviewing what I have done and for your suggestions.

Graham-72 commented 6 years ago

I have now found a way to modify the CKEditor plugin script so that the link text is available in the UI and can be customised by the user. My PR now includes the modifications.

Here are screenshots of what the UI now displays.

When starting to enter a new link
When editing an existing link
When starting to upload a file for linking

For comparison, here is the UI in Wordpress screenshot-2018-3-15 edit page graham s blog wordpress com

olafgrabienski commented 6 years ago

Testing the functionality in the sandbox site, I noticed two strange things:

I tried to add a local link which points to the About page. The result was a link to a sample txt file. Example: <a data-file-id="4" href="/files/uploads/sample_0.txt" linktext="Test link">. (In contrast, remote links like http://www.example.com do work as expected.)
When I changed the link text of a (remote) link via the "Edit link" modal, all the text in the editor except the linked term itself disappeared.

Graham-72 commented 6 years ago

@olafgrabienski thank you for testing this and reporting problems.

The first problem you found was not appearing on my own test site but I could see it on the sandbox. I have now created a new sandbox and the problem is not there at the moment.

The second problem I can see and it requires further changes in the JavaScript plugin but I am having difficulty with this. The 'Display Text' feature exists in the standard CKEditor Link plugin and seems to work well so it ought to be possible to include it in our Backdrop plugin.

So this PR is not yet final.

olafgrabienski commented 6 years ago

@Graham-72 You're welcome! Tested again, and I'm not able to reproduce the first problem in the new sandbox, so that one seems to be fixed. There are some PHP notices on the Recent log messages page, though, cf. admin/reports/dblog.

Graham-72 commented 6 years ago

I have now modified the PR to fix various problems and to remove the temporary attribute 'linktext' from the resulting link. It is now possible to edit an existing link to change it from a link to an existing page to be a link to a file, and vice versa. A remaining issue is that it is not possible to select a link and then insert text before or after it because such text becomes part of the link. I am not sure whether this can be fixed by further modification of the plugin script.

olafgrabienski commented 6 years ago

When I changed the link text of a (remote) link via the "Edit link" modal, all the text in the editor except the linked term itself disappeared.

@Graham-72 I confirm that the above mentioned issue is fixed now in the PR sandbox site.

I was also able to edit an existing link and change it from being a page link to be a file link, and vice versa.

Graham-72 commented 6 years ago

I have found that the style settings for the filter dialog need some adjustment, particularly for when the site's appearance setting is to use the site theme rather than the administration theme for editing or creating content. This does of course depend on the text sizes set in the site theme. I have added another commit to my PR.

jenlampton commented 6 years ago

The more I test this, the more I really like this feature.

I took a look at the PR and it looks like we decided that we needed to add a separate permission for adding files? I am on board with that approach.

edit: It looks like the new permission 'upload editor files' was included in the PR! That means this is RTBC from me :)

quicksketch commented 6 years ago

This looks really good and works great! I only found one major issue in my code review (and several smaller ones). Review posted to https://github.com/backdrop/backdrop/pull/2103#pullrequestreview-116510560.

quicksketch commented 6 years ago

@Graham-72 posted an update to the PR that fixes the only major issue I found. I'll give this one more pass, but I think this is now RTBC.

quicksketch commented 6 years ago

I merged https://github.com/backdrop/backdrop/pull/2103 into 1.x for 1.10. Thanks everyone here for all the collaboration (after we got past our slow start). Super-kudos to @Graham-72 for making everyone see the value AND putting in all the effort to build the implementation!

Graham-72 commented 6 years ago

I'm so pleased that this is merged! 👍

olafgrabienski commented 6 years ago

@Graham-72 I just noticed that there is no help text about the allowed file types in the "Upload a file and link to it" part of the link dialog. Should we add the information (in a follow-up issue)?

Graham-72 commented 6 years ago

@olafgrabienski good point. Something to think about tomorrow. Also, I am having a problem with adding an external URL and I think we need to make clear that it needs the http:// prefix. (Or doesn't it?)

backdrop / backdrop-issues

[UX] Allow uploading a file from the link dialog in rich-text editor #2072