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

Allows keeping values for font-size/family when pasting #61

Merged
merged 19 commits into from
Mar 25, 2020
Merged

Conversation

pomek
Copy link
Member

@pomek pomek commented Mar 9, 2020

Suggested merge commit message (convention)

Feature: Introduced an option for both (FontSize and FontFamily) plugins that allow keeping font-family and font-size values when pasting a text. Closes ckeditor/ckeditor5#6165. Closes ckeditor/ckeditor5#2278.


Additional information

TODO: Enable the features in "Paste from ..." demos.

@pomek
Copy link
Member Author

pomek commented Mar 9, 2020

@Reinmar, what should we do if a user used this new option in the FontSize plugin in an editor with presets that were using class names? I wrote some code that covers this case but I am not sure whether it is correct (according to ckeditor/ckeditor5#6165 (comment)).

@coveralls
Copy link

coveralls commented Mar 9, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 5af71e7 on i/6165 into b97e6a1 on master.

@pomek
Copy link
Member Author

pomek commented Mar 10, 2020

A few tests fail, trying to fix that.

docs/features/font.md Outdated Show resolved Hide resolved
@@ -49,7 +49,8 @@ export default class FontFamilyEditing extends Plugin {
'Times New Roman, Times, serif',
'Trebuchet MS, Helvetica, sans-serif',
'Verdana, Geneva, sans-serif'
]
],
disableValueMatching: false
Copy link
Member

Choose a reason for hiding this comment

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

TODO find a better name.

editor.conversion.for( 'downcast' ).attributeToElement( {
model: FONT_FAMILY,
view: ( attributeValue, writer ) => {
return writer.createAttributeElement( 'span', { style: 'font-family:' + attributeValue }, { priority: 7 } );
Copy link
Member

Choose a reason for hiding this comment

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

Why priority 7?

Copy link
Member

Choose a reason for hiding this comment

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

OK, it was needed to correctly sort the font color and font family and font size styles so they render in a visually correct way. But we need a test for that. Most likely, such a test exist for the previous converter, so it's a matter of copying it.

@@ -47,6 +47,25 @@ ClassicEditor

{@snippet features/custom-font-family-options}

### Prevent removing non-specified values

By default, all not specified values of `font-family` are removing during the conversion. You can disable this behaviour using `disableValueMatching` option.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
By default, all not specified values of `font-family` are removing during the conversion. You can disable this behaviour using `disableValueMatching` option.
By default, all not specified values of `font-family` are removing during the conversion. You can disable this behaviour using `disableValueMatching` option.

@Reinmar Reinmar merged commit b22efec into master Mar 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants