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

#7330 Editing part for the linking images feature #7386

Merged
merged 10 commits into from
Jun 10, 2020
Merged

#7330 Editing part for the linking images feature #7386

merged 10 commits into from
Jun 10, 2020

Conversation

pomek
Copy link
Member

@pomek pomek commented Jun 5, 2020

Suggested merge commit message (convention)

Feature (link): Introduced the editing part of linking images feature. Closes #7330.

Other (image): image/image/utils~getViewImgFromWidget() will look for nested <img> element in the widget.

☝️ I'm not sure it's worth mentioning.


Since the PR introduced only a part of the entire plugin (we're still missing UI and Docs), I'd suggest reviewing this code and make it as a base for other parts. New branches will be merged to this one and after finishing the entire plugin, we will be able to merge full stuff to the master.

CI: https://travis-ci.org/github/ckeditor/ckeditor5/builds/694996103

@pomek pomek requested a review from jodator June 5, 2020 11:55
Copy link
Contributor

@jodator jodator left a comment

Choose a reason for hiding this comment

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

Minor things to change:

  1. A small refactor in command to make it more compact.
  2. Converters are usually as functions.

packages/ckeditor5-link/src/linkcommand.js Outdated Show resolved Hide resolved
packages/ckeditor5-link/src/linkcommand.js Outdated Show resolved Hide resolved
packages/ckeditor5-link/src/linkimageediting.js Outdated Show resolved Hide resolved
@pomek
Copy link
Member Author

pomek commented Jun 10, 2020

@jodator, I applied your suggestion. Ready for review once again.

@jodator jodator merged commit cc0e694 into master Jun 10, 2020
@jodator jodator deleted the i/7330 branch June 10, 2020 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow linking images - editing
2 participants