LPS-116193 - As a developer I can specify custom tokens so that the style editor provides a form to edit those tokens instead of the Liferay-standard ones

[ruben-pulido]

This is a follow-up PR of https://github.com/jbalsas/liferay-portal/pull/2218

I have tried to incorporate all the feedback received in the previous PR:

This PR contains:

• A second draft version of the tokens definition JSON Schema file

• An example tokens definition JSON file following the tokens definition JSON schema.

• A token values JSON Schema file

• An example token values JSON file following the token values JSON Schema
  (I think it makes sense to start thinking already about how the token values JSON file should look like)

Summary of the most important changes:

• Flattened structure replacing the hierarchical structure following. I agree with @jbalsas this will give us more flexibility for introducing changes (@jbalsas, @izaera)
• Attribute cssVariable has been replaced by a more generic mappings attribute. I have tried to illustrate how a mapping for a CSS Style Template would look like here (@jbalsas, @izaera)
• An optional validValues attribute has been added (@jbalsas, @JorgeFerrer)
• Attribute type has been split into type (with possible values Boolean, Integer, Number, String) and editorType (for now only with possible values ColorPicker). (@izaera)
• Labels have been simplified to "label": "key (@jbalsas, @izaera)
• tokenSetCategories has been renamed to TokenCategories (@JorgeFerrer)
• "type": "object" has been explicitly added in the cases where it was missing (@wincent )
• The list of required attributes has been revisited (@wincent )
• The repeatable attribute has been removed for now for simplicity

Opened questions to clarify:

• CSSVariablesProviders are expected to return a collection of ScopedCSSVariables objects, each of which has a scope.
  Would it make sense to include outputScope as an additional property of a Mapping item?

Any additional feedback is also greatly appreciated!

//cc @ealonso @victorg1991

[liferay-continuous-integration]

To conserve resources, the PR Tester does not automatically run for every pull.

If your code changes were already tested in another pull, reference that pull in this pull so the test results can be analyzed.

If your pull was never tested, comment "ci:test" to run the PR Tester for this pull.

[jbalsas]

CSSVariablesProviders are expected to return a collection of ScopedCSSVariables objects, each of which has a scope.
Would it make sense to include outputScope as an additional property of a Mapping item?

I don't think so...

You can only know the scope when you apply a set of variables to a specific element (a site, a page, a widget, a fragment...), so defining the scope is a responsibility of the Application that wires them.

2 examples:

• The StyleBook app allows you to configure the Style Book applied to a Layout. The CSSVariablesProvider in that app will return a scope of body or #main-content or something like that since it knows that's where the token values need to apply.
• The PageEditor app (could) allow you to apply values to specific fragments in a page. The CSSVariablesProvider in there will return a scope of #${fragmentId} for each fragment that contains custom token values so that each configured set applies to each specific fragment only.

Does this make sense, @ruben-pulido? Please, let me know if this doesn't answer your question or I missed something

[ruben-pulido]

CSSVariablesProviders are expected to return a collection of ScopedCSSVariables objects, each of which has a scope.
Would it make sense to include outputScope as an additional property of a Mapping item?

I don't think so...

You can only know the scope when you apply a set of variables to a specific element (a site, a page, a widget, a fragment...), so defining the scope is a responsibility of the Application that wires them.

2 examples:

• The StyleBook app allows you to configure the Style Book applied to a Layout. The CSSVariablesProvider in that app will return a scope of body or #main-content or something like that since it knows that's where the token values need to apply.
• The PageEditor app (could) allow you to apply values to specific fragments in a page. The CSSVariablesProvider in there will return a scope of #${fragmentId} for each fragment that contains custom token values so that each configured set applies to each specific fragment only.

Does this make sense, @ruben-pulido? Please, let me know if this doesn't answer your question or I missed something

@jbalsas Thanks for the clarification and the illustrative examples. It makes sense.

[ruben-pulido]

@jbalsas I've updated the branch:

• Removing the mappings section for now as discussed
• Fixing the wrong value for the token type that was pointed out here

[jbalsas]

Thanks @ruben-pulido!

We will use the provided sample to Provide TokenDefinition for Classic Theme. We will see how to better include mappings in there and send it as a proposal over here so we can complete the schema. Hope that works!

We're also waiting on @izaera to re-submit the token-definition modules so we can house the schemas in there. @izaera, maybe you could copy these in there already, so we don't need to do it in an additional step and that way it'd be less likely for Brian to discard it afterwards?

/cc @marcoscv-work, @eduardoallegrini, @esanzp, let's prioritize an initial TokenDefinition for Classic so we can get this in asap! 😘

[ruben-pulido]

Mappings added back to the token definition JSON schema as discussed here

[jbalsas]

Thanks @ruben-pulido! #111 includes the token_definition schema but not the token_values.

We assumed those would go somewhere else. Can you confirm? If so, I think we can close this one since at least everything's been discussed?

[ruben-pulido]

@jbalsas We'll have to think where to store the token values JSON schema, but we can do that at a later point as part of another task/story.
@victorg1991 When working on the editor we can store the values according to token_values_json_schema_draft_v2.json and possibly then find a place to store it

I am OK with closing this issue

[jbalsas]

Maybe attach the additional json schemas and samples to the JIRA ticket so we don't lose them?

The scope of LPS-116193 might grant that we only get this in this part and leave the values definition for wherever you work on the storage part.

[jbalsas]

Closing here, then! Thanks for the work on this @ruben-pulido! (still waiting on brianchandotcom#91387 to be reviewed and merged, though)

[ruben-pulido]

(still waiting on brianchandotcom#91387 to be reviewed and merged, though)

@jbalsas I left a comment on brianchandotcom#91387 since the token-definition.json file and the JSON schema are out of sync there. The token-definition.json file needs to be updated.

[jbalsas]

🙈

@izaera, @eduardoallegrini, can you guys please coordinate and update to send a new PR?