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

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

Closed
jenlampton opened this issue Aug 18, 2016 · 50 comments
Closed

Comments

@jenlampton
Copy link
Member

jenlampton commented Aug 18, 2016

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 backdrop/backdrop#2082
Improved PR (also from Graham-72) now available for review backdrop/backdrop#2103

@Graham-72
Copy link

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.

@jenlampton jenlampton changed the title [UX] Allow a a link-to-file in the rich text editor [UX] Allow a link-to-file in the rich text editor Aug 19, 2016
@olafgrabienski
Copy link

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
Copy link
Member

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

@quicksketch
Copy link
Member

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...

@quicksketch quicksketch reopened this Mar 23, 2017
@quicksketch quicksketch changed the title [UX] Allow a link-to-file in the rich text editor [UX] Allow uploading a file from the link dialog in rich-text editor Mar 23, 2017
@jackaponte
Copy link

+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
Copy link

Graham-72 commented Apr 12, 2017

+1 from me too.

@Al-Rozhkov
Copy link
Member

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
Copy link
Member Author

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
Copy link

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
Copy link

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
screenshot-2018-1-27 edit page test page cgwebdesign

@olafgrabienski
Copy link

@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
Copy link

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
Copy link

olafgrabienski commented Apr 5, 2018

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
Copy link

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
Copy link
Member Author

jenlampton commented May 1, 2018

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
Copy link
Member

This looks really good and works great! I only found one major issue in my code review (and several smaller ones). Review posted to backdrop/backdrop#2103 (review).

@quicksketch
Copy link
Member

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

I merged backdrop/backdrop#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
Copy link

I'm so pleased that this is merged! 👍

@olafgrabienski
Copy link

@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
Copy link

@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?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants