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

For each major plugin specify the HTML output and add it to the package matadata #9401

Closed
ma2ciek opened this issue Apr 1, 2021 · 10 comments
Assignees
Labels
squad:devops Issue to be handled by the Devops team. type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@ma2ciek
Copy link
Contributor

ma2ciek commented Apr 1, 2021

Extracted from #6642

Scope

  • Try to document a couple of more "interesting" plugins with the proposed (or similar) format.
  • Review the outcome of the above.
  • Once we have the final decision, document other features.

Notes

Before documenting every plugin output, let's do it for a few to check if the proposal works.

HTML output

  • To not repeat ourselves - plugins are configurable and so is their output. Also, plugins influence one another (e.g. alignment + block features).
  • Conclusions:
    • It'd be enough if CKE5 documented in its docs (in one place) all the possible outputs of all the features
    • CKE5 could try to standardize where this information can be written down by 3rd party plugin authors

Current proposal:

"ckeditor": {
    "plugins": [
      { 
        "name": "Alignment",
        "output": [
          {
            "element": "$block",
            "attributes": [
              {
                "name": "style",
                "description": "For default config.", // Consider renaming to "comments"

                // Possible future extension - list values/particular styles
              },
              {
                "name": "class",
                "description": "For config.alignment.classes", // Consider renaming to "comments"

                // Possible future extension - list particular classes
              }
            ]
          }
        ]
      },
      { 
        "name": "Image",
        "output": [
          {
            "element": "figure",
            "attributes": [
              {
                "name": "class",
                "value": "image"
              }
            ]
          },
          {
            "element": "img",
            "attributes": [ "src", "srcset", "sizes" ]
          }
        ]
      },
@ma2ciek ma2ciek added type:task This issue reports a chore (non-production change) and other types of "todos". squad:devops Issue to be handled by the Devops team. squad:dx labels Apr 1, 2021
@Reinmar Reinmar added this to the iteration 42 milestone Apr 6, 2021
@psmyrek
Copy link
Contributor

psmyrek commented Apr 7, 2021

I would like to propose a slightly different output format, loosely modeled on MatcherPattern, where classes and styles are defined separately from other attributes.

{
    "plugin": "String",
    "output": [
        {
            "elements": "String | Array.<String>",
            "classes": "String | Array.<String>",
            "styles": "String | Array.<String>",
            "attributes": "String | Array.<String>",
            "_comment": "String"
        }
    ]
}

Fields description:

  • plugin contains plugin name.
  • output is an array of view elements, which are generated by this plugin. Each item in this array is an object with the following keys:
    • elements is a single element name or an array of element names, that are generated from given plugin. If elements is an array, then all other fields in this particular object (classes, styles, attributes and _comment) apply to all names from elements array.
    • classes is a string or an array of strings containing class names.
    • styles is a string or an array of strings containing style names.
    • attributes is a string or an array of strings containing attribute names other than class and style.
    • _comment contains an additional description.

Output for Table

Show
{
    "plugin": "Table",
    "output": [
        {
            "elements": "figure",
            "classes": "table",
            "styles": [
                "float",
                "height",
                "width"
            ]
        },
        {
            "elements": "table",
            "styles": [
                "background-color",
                "border-bottom",
                "border-left",
                "border-right",
                "border-top"
            ]
        },
        {
            "elements": [
                "thead",
                "tbody",
                "tr"
            ]
        },
        {
            "elements": [
                "td",
                "th"
            ],
            "styles": [
                "background-color",
                "border-bottom",
                "border-left",
                "border-right",
                "border-top",
                "height",
                "padding",
                "text-align",
                "vertical-align",
                "width"
            ],
            "attributes": [
                "colspan",
                "rowspan"
            ]
        }
    ]
}

From the "output" point of view, the <thead>, <tbody> and <tr> elements are the same (only the name differs), because they do not have any attributes, classes or styles. Similarly, the <td> and <th> elements are the same (except the name of course), because they all may contain the same attributes, classes or styles. This is why elements field in the output definition may be either a string, or an array of strings to group the same elements together.

@Reinmar @niegowski @ma2ciek Could you please review this new output format proposal?

@Reinmar
Copy link
Member

Reinmar commented Apr 7, 2021

  • _comment contains an additional description.

👍👍👍 for underscoring this

In general, the format looks interesting. I think we could try to go this way. Just some random thoughts:

  • Initially, I wasn't sure whether the classes/attrs/styles are meant to apply to these specified elements or it's "this feature allows these elements and some attributes somewhere". But I think it's something that can be documented and once you have the confirmation of what's the semantics, it's ok.
  • How would text alignment feature be documented? It allows "some classes" but we don't know their names (as this is dynamic).

@psmyrek
Copy link
Contributor

psmyrek commented Apr 7, 2021

The output format currently contains all possible classes, styles and attributes that the given plugin generates. I don't know if we want to go in the direction that we will additionally define classes, styles and attributes that are always present at the output?

In the case of a plugin, that creates elements with dynamic classes (e.g. Alignment plugin), we can use some wildcard character, e.g. "classes": "*" and describe this case in the _comment field.

@niegowski
Copy link
Contributor

  • Initially, I wasn't sure whether the classes/attrs/styles are meant to apply to these specified elements or it's "this feature allows these elements and some attributes somewhere". But I think it's something that can be documented and once you have the confirmation of what's the semantics, it's ok.

I had the same confusion at first.

  • How would text alignment feature be documented? It allows "some classes" but we don't know their names (as this is dynamic).

I think we don't need to specify exact class names and styles, we could put only 'class' and 'style' to the attributes list, this should be sufficient.

Don't we need to put comments on the specific attributes? 

The table example seems to include the TableProperies and TableCellProperties but those are not included in the Table plugin, they are separate features built on top of the Table and they require to be added separately.

@Reinmar
Copy link
Member

Reinmar commented Apr 7, 2021

Don't we need to put comments on the specific attributes?

I think that if a specific attribute depends on a specific e.g. config option, then it could be listed in a separate item of the output array. So if you have two attrs allowed by one feature and one is dependent and the other not, then you have two items of the output array.

I had the same confusion at first.

I'd add to that one thing: how do we differentiate allowing an element and allowing an attribute of an element.

Example: Table vs TableProperties features. Both should be documented separately.

@niegowski
Copy link
Contributor

I think that if a specific attribute depends on a specific e.g. config option, then it could be listed in a separate item of the output array. So if you have two attrs allowed by one feature and one is dependent and the other not, then you have two items of the output array.

Makes sense.

I'd add to that one thing: how do we differentiate allowing an element and allowing an attribute of an element.

This is a declaration of possible output so I'm not sure if we need to differentiate it

@niegowski
Copy link
Contributor

Also we need some way to indicate that some element is a $block, maybe "implements": "$block" would be a good notation?

@Reinmar
Copy link
Member

Reinmar commented Apr 7, 2021

Paragraph:

elements: 'p',
implements: '$block'

Alignment:

elements: '$block',
classes: '*'
_comments: '...'

Table:

elements: 'table'

TableProperties:

elements: 'table', // Not allowed by TableProps itself, but it's ok to keep it ambiguous
styles: [
	'border',
	'border-*'
]

@psmyrek
Copy link
Contributor

psmyrek commented Apr 16, 2021

ma2ciek added a commit that referenced this issue Apr 16, 2021
Internal: Add plugins metadata. Closes #9400. Closes #9401.
@ma2ciek
Copy link
Contributor Author

ma2ciek commented Apr 16, 2021

Closed by #9473.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad:devops Issue to be handled by the Devops team. type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

No branches or pull requests

4 participants