-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Introduce ViewElementConfigDefinition interface and utility classes #4216
Comments
About preliminary implementation in font size feature: |
There's already
Makes sense. I used singular nouns because they make more sense in configuration for that specific feature. But would be dealbreakers in general.
May be. It only depends on peoples' use cases which we cannot imagine now. But we can say that there's a pretty big chance that other attribute names will be used. |
I'm also thinking that perhaps a better name would be |
I'd go with this as part of https://github.com/ckeditor/ckeditor5-font/issues/2 since I've already half of it done there. I will also introduce this in heading feature. |
WDYM? Separate changes need to land in ckeditor5-engine, so it's a pretty much unrelated ticket. |
OK so maybe it wasn't clear (as I thought it was). I'd like to go with this ticket now to implement this in header and then finish ckeditor5-font/2. As for now I don't see much work for this ticket alone besides docs. It might shed some light on utility methods when I'll make changes in those 2 features (font and heading). To have some idea's how this interface going to be used in other words. As for PR's it rather be two unrelated PRs: one in engine and following in font feature. |
PS. The feature using |
It makes sens to have common syntax to create view/DOM elements in the simpler way. The one proposed above, with However, I think that we should not go too far. |
We won't be able to use these options without support for |
Since // an array of matcher patterns
view: {
to: { name, class, style, attribute }
from: [
{ name, class, style, attribute },
{ name, class, style, attribute },
//...
]
}
// or a callback for most complex cases:
view: {
to: { name, class, style, attribute }
from: () => {}
}
// or when defined as one object it will assume view.to=== view.from:
view: {
name,
class,
style,
attribute
} as converting M->V will always create the same result but when parsing content it might be required to convert different elements to same model. |
Probably support for priorities makes sense also: const headingConfig = {
options: [
{ model: 'paragraph', title: 'paragraph' },
{
model: 'heading1',
view: {
from: [
{ name: 'h1' },
{ name: 'p', attribute: { 'data-heading': 'h1' }, priority: 'high' }
],
to: {
name: 'h1'
}
},
title: 'User H1'
}
]
} would allow to convert such HTML: <h1>foobar</h1>
<p data-heading="h1">foobar</p>
<p>Other text</p> |
As much as I don't like declarative APIs (in general), I have no better idea in this case. However I think that |
@pjasiun: after some quick thinking until better name appears I'll go with const headingConfig = {
options: [
// 1: straight forward: any 'h1' element:
{
model: 'heading1',
view: 'h1'
},
// 2: a bit narrower: any 'h2' element with 'heading-foo' class
{
model: 'headingFoo',
view: {
name: 'h2',
class: 'heading-foo' // also as an array: [ 'foo', 'bar' ]
// would also accept: 'attribute', `style` objects
}
},
// 3: Full flavor with other elements parsed as 'headingBar':
{
model: 'headingBar',
view: 'h3',
converts: [
{ name: 'p', attribute: { 'data-heading': 'h3' } },
{ name: 'p', class: [ 'heading', 'heading-lvl-2' ] },
{ name: 'h4', style: { 'font-size': '23em' } }
]
}
]
} as an alternative to option 3 we might define view in full-flavor as: {
output: { name: 'h1' },
converts: [
{ name: 'h2' },
{ name: 'h3', attribute: { 'data-heading': 'lvl1' } }
]
} ps.: |
I'd go with |
No opinion. Both seem good to me at the moment. |
I'm for the first one. The second looks like there is |
Feature: Introduced `ViewElementDefinition` and `definition-based-converters` module with a set of utils allowing to turn element definitions to converters. Closes #1198.
This gathers requirements from both heading and https://github.com/ckeditor/ckeditor5-font/issues/2#issuecomment-347838197 features.
It was proposed by @Reinmar in here.
The
ViewElementConfigDefinition
would server as anview
configuration for some features that requires flexibility in parsing and should allow to greater customization of editor features like Heading and FontSize.Full structure (whether values makes sense or not):
This would allow to:
Things to consider:
class
andstyle
rather thenstyles
andclasses
- such forms are already used in other places AFAICS.data-header=true
) needed?Ad. 2. Examples:
This might grow heavy so alternative configuration would by just strings with defined format:
As for second flavour we already have utility methods for parsing those:
https://github.com/ckeditor/ckeditor5-engine/blob/8eda4e5308f192cee58dd3d4b425f42769f66f17/src/view/element.js#L728
https://github.com/ckeditor/ckeditor5-engine/blob/8eda4e5308f192cee58dd3d4b425f42769f66f17/src/view/element.js#L804
The text was updated successfully, but these errors were encountered: