Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

T/1: The basic media embed feature #2

Merged
merged 47 commits into from
Aug 24, 2018
Merged

T/1: The basic media embed feature #2

merged 47 commits into from
Aug 24, 2018

Conversation

oleq
Copy link
Member

@oleq oleq commented Aug 20, 2018

Suggested merge commit message (convention)

Feature: The basic media embed feature. Closes ckeditor/ckeditor5#2727.


Additional information

Requires:

Followups (in progress...):

oleq added 28 commits August 20, 2018 11:25
@oleq
Copy link
Member Author

oleq commented Aug 22, 2018

Don't images feel overwhelming too? My point is – I think that it's the same kind of content and perhaps should be styled consistently.

Fixed in 0388d39.

@oleq
Copy link
Member Author

oleq commented Aug 22, 2018

The media widget is inserted always after the block in which you currently have the caret. The image insertion works a bit better and if you have the caret at the beginning of that block it gets inserter before it. This can be a followup, of course.

A follow-up in https://github.com/ckeditor/ckeditor5-media-embed/issues/12.

.travis.yml Outdated
@@ -1,7 +1,7 @@
sudo: required
dist: trusty
addons:
firefox: latest
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this. We unified the configs to not use " if not necessary (that's what travis encrypt does too).

Copy link
Member Author

Choose a reason for hiding this comment

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

We unified the configs to not use " if not necessary

And almost every repo uses "". I'm confused, should I use it or not?

(t/ckeditor5-media-embed/1 fcea342) oleq@MBP ~/ck/5/ckeditor5> mgit exec 'cat ./.travis.yml | grep firefox'
 #  @ckeditor/ckeditor5-adapter-ckfinder                                    1/38 (3%)
firefox: "latest"

 #  @ckeditor/ckeditor5-alignment                                           2/38 (5%)
firefox: "latest"

 #  @ckeditor/ckeditor5-autoformat                                          3/38 (8%)
firefox: "latest"

 #  @ckeditor/ckeditor5-autosave                                           4/38 (11%)
firefox: "latest"

 #  @ckeditor/ckeditor5-basic-styles                                       5/38 (13%)
firefox: "latest"

 #  @ckeditor/ckeditor5-block-quote                                        6/38 (16%)
firefox: "latest"

 #  @ckeditor/ckeditor5-cloud-services                                    11/38 (29%)
firefox: "latest"

 #  @ckeditor/ckeditor5-clipboard                                         12/38 (32%)
firefox: "latest"

 #  @ckeditor/ckeditor5-easy-image                                        13/38 (34%)
firefox: "latest"

 #  @ckeditor/ckeditor5-core                                              14/38 (37%)
firefox: "latest"

 #  @ckeditor/ckeditor5-editor-balloon                                    15/38 (39%)
firefox: "latest"

 #  @ckeditor/ckeditor5-editor-classic                                    16/38 (42%)
firefox: "latest"

 #  @ckeditor/ckeditor5-editor-inline                                     17/38 (45%)
firefox: "latest"

 #  @ckeditor/ckeditor5-editor-decoupled                                  18/38 (47%)
firefox: latest

 #  @ckeditor/ckeditor5-engine                                            19/38 (50%)
firefox: "latest"

 #  @ckeditor/ckeditor5-enter                                             20/38 (53%)
firefox: "latest"

 #  @ckeditor/ckeditor5-essentials                                        21/38 (55%)
firefox: "latest"

 #  @ckeditor/ckeditor5-font                                              22/38 (58%)
firefox: latest

 #  @ckeditor/ckeditor5-heading                                           23/38 (61%)
firefox: "latest"

 #  @ckeditor/ckeditor5-highlight                                         24/38 (63%)
firefox: latest

 #  @ckeditor/ckeditor5-image                                             25/38 (66%)
firefox: "latest"

 #  @ckeditor/ckeditor5-link                                              26/38 (68%)
firefox: "latest"

 #  @ckeditor/ckeditor5-list                                              27/38 (71%)
firefox: "latest"

 #  @ckeditor/ckeditor5-markdown-gfm                                      28/38 (74%)
firefox: "latest"

 #  @ckeditor/ckeditor5-paragraph                                         29/38 (76%)
firefox: "latest"

 #  @ckeditor/ckeditor5-media-embed                                       30/38 (79%)
firefox: "latest"

 #  @ckeditor/ckeditor5-table                                             31/38 (82%)
firefox: latest

 #  @ckeditor/ckeditor5-theme-lark                                        32/38 (84%)
firefox: "latest"

 #  @ckeditor/ckeditor5-typing                                            33/38 (87%)
firefox: "latest"

 #  @ckeditor/ckeditor5-ui                                                34/38 (89%)
firefox: "latest"

 #  @ckeditor/ckeditor5-undo                                              35/38 (92%)
firefox: "latest"

 #  @ckeditor/ckeditor5-upload                                            36/38 (95%)
firefox: "latest"

 #  @ckeditor/ckeditor5-utils                                             37/38 (97%)
firefox: "latest"

 #  @ckeditor/ckeditor5-widget                                           38/38 (100%)
firefox: "latest"

Copy link
Member

Choose a reason for hiding this comment

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

Mhm.

 #  @ckeditor/ckeditor5-alignment                                           1/37 (3%) 
firefox: latest

 #  @ckeditor/ckeditor5-adapter-ckfinder                                    2/37 (5%) 
firefox: latest

 #  @ckeditor/ckeditor5-autosave                                            3/37 (8%) 
firefox: latest

 #  @ckeditor/ckeditor5-autoformat                                         4/37 (11%) 
firefox: latest

 #  @ckeditor/ckeditor5-basic-styles                                       5/37 (14%) 
firefox: latest

 #  @ckeditor/ckeditor5-block-quote                                        6/37 (16%) 
firefox: latest

 #  @ckeditor/ckeditor5-build-balloon                                      7/37 (19%) 
cat: ./.travis.yml: No such file or directory

 #  @ckeditor/ckeditor5-build-classic                                      8/37 (22%) 
cat: ./.travis.yml: No such file or directory

 #  @ckeditor/ckeditor5-build-inline                                       9/37 (24%) 
cat: ./.travis.yml: No such file or directory

 #  @ckeditor/ckeditor5-build-decoupled-document                          10/37 (27%) 
cat: ./.travis.yml: No such file or directory

 #  @ckeditor/ckeditor5-clipboard                                         11/37 (30%) 
firefox: latest

 #  @ckeditor/ckeditor5-cloud-services                                    12/37 (32%) 
firefox: latest

 #  @ckeditor/ckeditor5-core                                              13/37 (35%) 
firefox: latest

 #  @ckeditor/ckeditor5-easy-image                                        14/37 (38%) 
firefox: latest

 #  @ckeditor/ckeditor5-editor-balloon                                    15/37 (41%) 
firefox: latest

 #  @ckeditor/ckeditor5-editor-classic                                    16/37 (43%) 
firefox: latest

 #  @ckeditor/ckeditor5-editor-decoupled                                  17/37 (46%) 
firefox: latest

 #  @ckeditor/ckeditor5-editor-inline                                     18/37 (49%) 
firefox: latest

 #  @ckeditor/ckeditor5-enter                                             19/37 (51%) 
firefox: latest

 #  @ckeditor/ckeditor5-engine                                            20/37 (54%) 
firefox: latest

 #  @ckeditor/ckeditor5-essentials                                        21/37 (57%) 
firefox: latest

 #  @ckeditor/ckeditor5-font                                              22/37 (59%) 
firefox: latest

 #  @ckeditor/ckeditor5-heading                                           23/37 (62%) 
firefox: latest

 #  @ckeditor/ckeditor5-highlight                                         24/37 (65%) 
firefox: latest

 #  @ckeditor/ckeditor5-link                                              25/37 (68%) 
firefox: latest

 #  @ckeditor/ckeditor5-image                                             26/37 (70%) 
firefox: latest

 #  @ckeditor/ckeditor5-markdown-gfm                                      27/37 (73%) 
firefox: latest

 #  @ckeditor/ckeditor5-list                                              28/37 (76%) 
firefox: latest

 #  @ckeditor/ckeditor5-paragraph                                         29/37 (78%) 
firefox: latest

 #  @ckeditor/ckeditor5-table                                             30/37 (81%) 
firefox: latest

 #  @ckeditor/ckeditor5-theme-lark                                        31/37 (84%) 
firefox: latest

 #  @ckeditor/ckeditor5-typing                                            32/37 (86%) 
firefox: latest

 #  @ckeditor/ckeditor5-ui                                                33/37 (89%) 
firefox: latest

 #  @ckeditor/ckeditor5-undo                                              34/37 (92%) 
firefox: latest

 #  @ckeditor/ckeditor5-upload                                            35/37 (95%) 
firefox: latest

 #  @ckeditor/ckeditor5-utils                                             36/37 (97%) 
firefox: latest

 #  @ckeditor/ckeditor5-widget                                           37/37 (100%) 
firefox: latest

Execution time: 2s489ms.

Copy link
Member

Choose a reason for hiding this comment

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

We unified this recently, Olek.

@Reinmar
Copy link
Member

Reinmar commented Aug 23, 2018

Why does removeProviders: [ 'youtube', 'twitter' ] remove a tweet from the content? Shouldn't it only block adding more of them?

@Reinmar
Copy link
Member

Reinmar commented Aug 23, 2018

Why does removeProviders: [ 'youtube', 'twitter' ] remove a tweet from the content? Shouldn't it only block adding more of them?

Merge state

OTOH, maybe it's a good idea to filter them out. Thanks to that, if someone pastes you some embed from one editor to another, you'll only get supported media in the latter editor.

@Reinmar
Copy link
Member

Reinmar commented Aug 24, 2018

One more issue - in the mediaembed.html manual test you have saved semantic HTML, not full HTML.

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

Successfully merging this pull request may close these issues.

Add a basic support for embedding the media
3 participants