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

Implement declarative image style drop-downs #9340

Closed
oleq opened this issue Mar 23, 2021 · 4 comments · Fixed by #9537 or #9236
Closed

Implement declarative image style drop-downs #9340

oleq opened this issue Mar 23, 2021 · 4 comments · Fixed by #9537 or #9236
Assignees
Labels
package:image squad:core Issue to be handled by the Core team. type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@oleq
Copy link
Member

oleq commented Mar 23, 2021

📝 Provide a description of the improvement

config.image.style.groups makes little sense because it has no impact on the model/data/upcast and general logic of the feature. It exists only to create dropdowns in the component factory. 

Instead of using this configuration, we can configure dropdowns declaratively, thus separating the model and view layers.

Related to #1472 (comment).


If you'd like to see this improvement implemented, add a 👍 reaction to this post.

@oleq oleq added type:improvement This issue reports a possible enhancement of an existing feature. package:image squad:core Issue to be handled by the Core team. labels Mar 23, 2021
@oleq oleq added this to the iteration 42 milestone Mar 23, 2021
@magda-chrzescian
Copy link
Contributor

@oleq @Reinmar This way we won't provide the default dropdown configuration for custom editor implementations (other than builds with predefined toolbars). So the developer won't be able to use toolbar: [ 'imageStyle:wrapText' ].

Also, the image.style.options that we provide also aren't separated from the view layer - it is used for creating the style buttons, so using image.style.groups for creating dropdowns seems consistent in some way.

@oleq
Copy link
Member Author

oleq commented Apr 6, 2021

I guess we can have some pre-defined drop-downs already registered in the component factory, such as 'imageStyle:wrapText' which can be referenced by their name. When the editor initialized, the image style feature will analyze the toolbar config and then:

  • populate the component factory using dropdown definitions,
  • and use pre-defined dropdowns by name.

A random thought: I just realized that to make declarative drop-downs work, they need to be registered in the component factory before the image toolbar is loaded (WidgetToolbarRepository calls ToolbarView#fillFromConfig() and that helper is based on the component factory). And since ImageToolbar is standalone and only required by the top-level editor configuration, this could be a problem (although it kicks in on afterInit so maybe not, this needs to be checked).

@magda-chrzescian
Copy link
Contributor

We discussed this issue with @oleq and we had some concerns about the place where the image.toolbar dropdown definition should be replaced with the name of the component registered in the ComponentFactory. It has to be done at some point because the ComponentFactory#add can handle the registered components' names only and should not be responsible for registering anything. We defined a few options:

  • To override the dropdowns definitions with their names directly in the ImageStyleUI - but we don't want to mutate the toolbar config.
  • To extract the dropdown name and handle it in the ToolbarView#fillFromConfig method.
  • To listen to the WidgetToolbarRepository#register or even ComponentFactory#create from the ImageStyleUI and register the dropdowns and override the declarations in the flow - so the ImageStyleUI wouldn't have to read the image.toolbar.
  • To normalize the declarative dropdowns definitions while creating the imageToolbar - which is the solution that we picked, for now, to encapsulate this functionality in the image package only.

We also explored some options of extending this feature to support declarative dropdowns globally. To allow the user to contain buttons from various plugins in a single dropdown, creating the dropdowns could be handled while initing the editor UI. It triggers after all of the plugins are already inited, so all of the buttons should be already registered in the ComponentFactory.

@magda-chrzescian magda-chrzescian linked a pull request Apr 21, 2021 that will close this issue
oleq added a commit that referenced this issue May 4, 2021
Feature (image): It should be possible to define the dropdown menu as an object in the `config.image.toolbar` configuration. Closes #9340.
@oleq
Copy link
Member Author

oleq commented May 4, 2021

Closed in #9537.

@oleq oleq closed this as completed May 4, 2021
oleq added a commit that referenced this issue Jun 1, 2021
Feature (image): Introduced support for inline images in editor content. Available out–of–the–box in all ready–to–use editor builds inline images can be uploaded, styled, resized, and linked and complement the already supported block images. See the [image feature overview guide](https://ckeditor.com/docs/ckeditor5/latest/features/images/images-overview.html) to see inline images in action. For more information about breaking changes, migration path, and tips, check out the [migration guide](https://ckeditor.com/docs/ckeditor5/latest/builds/guides/migration/migration-to-28.html).

Feature (image): It should be possible to define the dropdown menu as an object in the `config.image.toolbar` configuration. Closes #9340.

Other (image): Turned the image utils module into an editor plugin to allow sharing utils outside the package. See #8871.

Fix (image): The side-aligned images should always have some `min-width` property to not take the whole editor width. Closes #9342.

Fix (image): The floating block images, except the `side` image, should be displayed side by side by default. Closes #9183.

Fix (image): An image should never overflow the widget boundaries while changing its size. Closes #9166.

Fix (image): The size label should be displayed above the image if it doesn't fit inside. See #9166.

Fix (image): An Image caption placeholder text should not wrap or overflow. Closes #9162.

Other (image): The image toolbar should be visible if the selection is placed inside an image caption. Closes #9136.

Other (image): The image caption should be controlled using the toolbar button and a command for better integration with image styles. Closes #8907.

Other (build-decoupled-document): The editor document build should include the `ImageResize` plugin. Closes #9507.

Feature (link): Integrated the link feature with inline images. Closes #8871. Closes #9017. Closes #9167.

Fix (link): The link UI should be shown when clicking a linked inline widget. Closes #9607.

Other (widget): Safeguarded the way the `Widget` plugin sets the fake selection. Closes #9580.

Fix (widget): Selected inline widgets wrapped in an attribute in the view should create a fake selection. Closes #9524. Closes #9521.

Other (widget): Replaced the `findOptimalInsertionPosition()` helper with `findOptimalInsertionRange()` that will now attempt to replace selected blocks when inserting new widgets. Closes #9102.

Fix (clipboard): Hide all toolbars when the user starts dragging the widget and show them when the drag ends. Closes #9566.

Fix (code-block): The code block feature should not allow for inserting inline widgets as its content. Closes #9567.

Other (easy-image): Removed the `Image` plugin dependency from the `EasyImage` plugin. Closes #9399.

Other (table): Added the `.ck-table-bogus-paragraph` CSS class to the in-cell pseudo-paragraph used for data tables for easier and safer styling.

Other (core): Added several icons for new image styles (see #8909).

Other (ui): Added the `class` property to the `SplitButtonView` UI component. See #8909.

Other (image, table, html-embed, media-embed, page-break, horizontal-line): New widgets will replace the selected block instead of being added next to it while being inserted (see #9102).

BREAKING CHANGE (image): The `Image` plugin works as a glue for both `ImageBlock` and `ImageInline` features now (previously only block images). If you do not want inline images to be allowed, consider replacing the `Image` plugin with `ImageBlock` in your editor configuration. Otherwise, all images without the `<figure>` wrapper will be loaded into the editor content as inline images, which in some cases may affect content semantics and styling.

BREAKING CHANGE (image): The `ImageEditing` plugin is no longer standalone as the majority of its logic was extracted to `ImageBlockEditing` and `ImageInlineEditing` plugins. The logic remaining in the `ImageEditing` is common for both `ImageBlockEditing` and `ImageInlineEditing` plugins.

BREAKING CHANGE (image): The image caption is no longer displayed automatically the user selects a block image. Instead, its presence is controlled using the `'toggleImageCaption'` toolbar button and a [`ToggleImageCaptionCommand`](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_imagecaption_toggleimagecaptioncommand-ToggleImageCaptionCommand.html) for better integration with [revamped image styles system](https://ckeditor.com/docs/ckeditor5/latest/builds/guides/migration/migration-to-28.html#image-styles). [Learn more](https://ckeditor.com/docs/ckeditor5/latest/builds/guides/migration/migration-to-28.html#image-caption).

BREAKING CHANGE (image): Some helpers from the image utils module (`@ckeditor/ckeditor5-image/src/image/utils.js`) have moved to the [`ImageUtils` plugin](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_imageutils-ImageUtils.html). The helpers are still accessible via `editor.plugins.get( 'ImageUtils' )` namespace, for instance, `editor.plugins.get( 'ImageUtils' ).insertImage( ... )`.

BREAKING CHANGE (image): The API of the [`insertImage()`](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_imageutils-ImageUtils.html#function-insertImage) helper has changed, please make sure to update your integrations.

BREAKING CHANGE (image): The `getSelectedImageWidget()` helper is no longer part of the public API.

BREAKING CHANGE (image): The `getViewImgFromWidget()` helper is no longer part of the public API.

BREAKING CHANGE (image): The `isImageAllowed()` helper is no longer part of the public API.

BREAKING CHANGE (image): The `isImage()` helper is no longer part of the public API.

BREAKING CHANGE (image): The `isImageWidget()` helper is no longer part of the public API.

BREAKING CHANGE (image): The `toImageWidget()` helper is no longer part of the public API.

BREAKING CHANGE (image): The `captionElementCreator()` helper is no longer part of the public API.

BREAKING CHANGE (image): The `isCaption()` helper is no longer part of the public API.

BREAKING CHANGE (image): The `getCaptionFromImage()` helper is now available as [`getCaptionFromImageModelElement()`](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_imagecaption_utils.html#function-getCaptionFromImageModelElement).

BREAKING CHANGE (image): The `matchImageCaption()` helper is now available as [`matchImageCaptionViewElement()`](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_imagecaption_utils.html#function-matchImageCaptionViewElement).

BREAKING CHANGE (image): The `defaultIcons` are now available as [`DEFAULT_ICONS`](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_imagestyle_utils.html#constant-DEFAULT_ICONS).

BREAKING CHANGE (image): The `defaultStyles` are now available as [`DEFAULT_OPTIONS`](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_imagestyle_utils.html#constant-DEFAULT_OPTIONS).

BREAKING CHANGE (image): The linked image indicator (icon) rendered as a `<span>` with the `.ck-link-image_icon` CSS class has been removed. To alter the look of the indicator (including the icon), please use `figure.image > a::after` (for linked block images) and `a span.image-inline::after` (for linked inline images) CSS selectors instead.

BREAKING CHANGE (image): The `isImageWidget()` helper is no longer exposed by the [`Image`](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_image-Image.html) plugin.

BREAKING CHANGE (image): The API of common [image converters](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_image_converters.html) has changed. Please refer to the documentation to learn more.

BREAKING CHANGE (image): The API of various [image caption utils](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_imagecaption_utils.html) has changed. Please refer to the documentation to learn more.

BREAKING CHANGE (image): The conversion helpers [`srcsetAttributeConverter()`](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_image_converters.html#function-srcsetAttributeConverter) and [`modelToViewAttributeConverter()`](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_image_converters.html#function-modelToViewAttributeConverter) now require the `imageType` parameter.

BREAKING CHANGE (easy-image): The [`EasyImage`](https://ckeditor.com/docs/ckeditor5/latest/api/module_easy-image_easyimage-EasyImage.html) plugin is no longer automatically importing the [`Image`](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_image-Image.html) plugin as a dependency. [Learn more](https://ckeditor.com/docs/ckeditor5/latest/builds/guides/migration/migration-to-28.html#easyimage-plugin).

BREAKING CHANGE (ckfinder): The [`CKFinder`](https://ckeditor.com/docs/ckeditor5/latest/api/module_ckfinder_ckfinder-CKFinder.html) plugin is no longer automatically importing the [`Image`](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_image-Image.html) plugin as a dependency. [Learn more](https://ckeditor.com/docs/ckeditor5/latest/builds/guides/migration/migration-to-28.html).

BREAKING CHANGE (link): The `isImageAllowed()` helper is now available as [`isLinkableElement()`](https://ckeditor.com/docs/ckeditor5/latest/api/module_link_utils.html#function-isLinkableElement).

BREAKING CHANGE (media-embed): The API of the [`insertMedia()`](https://ckeditor.com/docs/ckeditor5/latest/api/module_media-embed_utils.html#function-insertMedia) helper has changed. See the documentation to learn more.

BREAKING CHANGE (build-decoupled-document): The official [decoupled document](https://www.npmjs.com/package/@ckeditor/ckeditor5-build-decoupled-document) build now ships with the [`ImageResize`](https://ckeditor.com/docs/ckeditor5/latest/api/module_image_imageresize-ImageResize.html) plugin enabled by default. [Learn more](https://ckeditor.com/docs/ckeditor5/latest/builds/guides/migration/migration-to-28.html).

BREAKING CHANGE (table): The in-cell pseudo-paragraph used for data tables is no longer styled using the inline `style` attribute but a `.ck-table-bogus-paragraph` CSS class instead.

BREAKING CHANGE (widget): The `findOptimalInsertionPosition()` helper is now `findOptimalInsertionRange()` and returns a [model range](https://ckeditor.com/docs/ckeditor5/latest/api/module_engine_model_range-Range.html). Also, instead of searching for a position next to the currently selected block, it will now attempt to replace that block (see #9102).

BREAKING CHANGE (widget): The `checkSelectionOnObject()` helper is no longer part of the public API.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:image squad:core Issue to be handled by the Core team. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
3 participants