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

RFC: HTML specs #4658

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

RFC: HTML specs #4658

wants to merge 2 commits into from

Conversation

kspeyanski
Copy link
Contributor

@kspeyanski kspeyanski commented Aug 14, 2023

The following PR aims to provide a structure and intention for the html .spec file, as well as scratch the surface of some long-term goals and address inconsistencies in current implementations.

I will go over the changes and try to explain them and put them into context, as well as how we expect them to affect the Kendo/Telerik products and the ThemeBuilder product.

We're starting with the Form!

1. Polymorphism

Introduce polymorphism into the components. I will take the Form for an example, where it was partially addressed. The Form specification has the following logic, which renders either form or div element:

const Parent = ({ tag, className, children }) => ( 
  tag === 'form' 
    ? <form className={className}>{children}</form> 
    : <div className={className}>{children}</div>
);

We're will try exposing the polymorphism to a first-level API through the introduction of the as property (though, per-framework implementation may varry). The main idea is to allow each of our components to render a different DOMElement or entirely custom component.

A simple showcase of the problem we're trying to solve here is the anchor button scenario.

Check out the following requirement:

Imagine you have a Kendo <Button /> component. Upon clicking it you would like to redirect to another page, just like a regular anchor.

Obviously handling the onClick and manually redirecting would do the job with javascript would do the job, but it's way over engineered solution for something so simple and common.

Instead, we should aim to provide the ability to override/extend the top-most DOMElement of the component. With the introduction of the as property would enable the following syntax:

<Button as="a" href="/next-page" >Link Button</Button>

Inside the component, we consume it in the following way:

const { as: Component = 'form', ...other } = props;

return <Component {...other} />

This would keep the component functional and styled accordingly, but explicitly changing its markup!

2. First-level APIs interface

In order to enforce all spec files to account for such behavior and implement it seamlessly, we're introducing a KendoComponent interface. It dictates the properties we associate with a spec file (the states, options, className and defaultProps) as well as the newly introduced polymorphic as prop. It accepts 2 generic fields, the default rendered element (e.g. div, span, form) and the properties of the component.

Please implement it as soon as possible during the development of a single spec:

export const Form: KendoComponent<"form", KendoFormProps> = () => {}

This would further unlock settings the following fields:

Form.states = states;
Form.options = options;
Form.className = FORM_CLASSNAME;
Form.defaultProps = defaultProps;

The interface enforces the implementation of all DOM attributes associated with the default rendered DOM element, so no attribute is missed (e.g. we've implemented onClick, but forget to implement onMouseDown). In addition, it works with the as property from the first point of the RFC to dynamically change the available DOM attributes.

3. KISS specs

Reduce spec's complexity. I will use the current form spec for an example. Here we're iterating over each children and enforcing them to be wrapped in a FormField wrapper. This is unnecessary, the form should not be override any of it's children and be responsible only for it's own rendering.

I suggest we move forward in the direction where the component are less restricted in terms of what can we rendered inside them, and instead provide guidance and documentation to provide best-practices and cover common scenarios instead of internally overriding the rendering - an implicit behavior not initially visible to the end user.

const formChildren: JSX.Element | JSX.Element[] = [];
if (children) {
if ( Array.isArray(children) ) {
children.map( (child, index) => {
if ( child.type === FormField ) {
formChildren.push(
<FormField {...child.props} orientation={orientation} key={index} />
);
} else {
formChildren.push(child);
}
} );
} else if ( children.type === FormField ) {
formChildren.push( <FormField {...children.props} orientation={orientation} key={`${new Date().getTime()}`} /> );
} else {
children.type === Fieldset && formChildren.push( <Fieldset {...children.props} key={`${new Date().getTime()}`} /> );
}
}

4. Standard extension points

Moving forward from the point above, we should be able to provide some meaningful points of extensions (in contrast of being all or nothing). This is a tricky point and should be fine-tuned on a per-component case, but we propose the usage of the term slot.

The end goal of this feature is to have a consistent, meaningful and standard functional and styling extension points and enforce it in all products trough the html specs.

A slot is a meaningful point of extensions inside an otherwise encapsulated component, both in terms of functionality and styling. Extending the example with the form, a meaningful point of extension for the Form spec would be the FormButtons component.
A slot can be identified by the following properties:

  • It's commonly rendered inside other component (every form need an additional div wrapper around it's buttons contaienr)
  • Has a unique className identifier (k-form-buttons)
  • Has additional styles attached to it (
    .k-form-buttons {
    @extend .k-actions !optional;
    padding: 0;
    overflow: visible;
    )

Other example for a slot can be:

  • The button inside a Picker
  • A row, cell or header of a Grid
  • A prefix/suffix of an input.

In the current RFC, a slot property is uppercased in order to be differentiated from a regular property, but this is subject to change.

Ultimately, a html spec's slot should only showcase intention to implement. Per-framework implementation should be executed according to the framework's tooling, e.g.:

@telerik/theme-builder-pro-team — Can we potentially utilize this API for the "Component Parts" section, and have a single source of which parts are customizable and which are not?

5. Styles-related

Removal of utility-based properties: Having a colX and gap property in the component's API might sound convenient at first, but my recommendation would be to not approach this problem through the JS API, the reasoning:

  • Scaling — We cannot implement all of the 250+ styling variants inside the components.
  • Coupling — its provides tight coupling between the css utility classNames with the components API.
  • Properties bloat — In-between functional properties, we've introduced some style-based properties which might not work with one-another (e.g. layout != grid + gapX prop).

Instead, my recommendation is to remove all styles-related API fields/props/attributes, and instead start utilizing the css classNames in sample apps, docs and demos and public resources.

So, instead of:

<Form gapX={4} />

we use:

<Form className={"k-gap-4"} />

Our css-utils documentation it relatively complete, so at this point, re-implementing the logic inside the components should be avoided due to the reasoning provided above.


@telerik/kendo-dev-leads, @telerik/kendo-front-end thoughts?

@Juveniel
Copy link
Contributor

1 ), 2 ), 3 ) - > No comments here, I think those are nice improvements to the spec.

4 ) -> Standardizing the templates approach across component suites can be quite hard, as currently we have cases where the template/slot allows customizing the entire element, and others where the template allows customizing the content of the element. However, the HTML spec can provide the guidance on which are the templates/slots that allow customization and what is the recommended* way of customizing them.

5 ) -> Decoupling the utilities package from the theme( component styles ) is definitely something that we should explore. However, deprecating positioning/alignment props that are already implemented in components could be a difficult task, as this would require first to transfer specific styles from utils package to the themes, and also take into consideration the impact for clients that currently rely on using the already implemented API options. I would suggest to have a separate discussion in regards to the future of the utilities package and its usage.

@JoomFX
Copy link
Contributor

JoomFX commented Aug 17, 2023

I also think that points 1,2 and 3 would be a nice improvement to the specs.
Just a quick clarification on point 3 - in some cases we iterate over the children in order to implement/modify something from a single place, otherwise we would need to go through and update many existing visual tests. The Toolbap spec is a good example:

if (child.type === Button && child.props.className && child.props.className.includes('k-toolbar-overflow-button')) {
tempToolbarChildren.push(
<Button
key={`${index}-${new Date().getTime()}`}
{...child.props}
className={`${child.props.className ? child.props.className : ''}`}
></Button>
);
} else if (child.type === Button && child.props.className && child.props.className.includes('k-toggle-button')) {
tempToolbarChildren.push(
<Button
key={`${index}-${new Date().getTime()}`}
{...child.props}
className={`k-toolbar-toggle-button ${child.props.className ? child.props.className : ''}`}
></Button>
);
} else if (child.type === Button) {
tempToolbarChildren.push(
<Button
key={`${index}-${new Date().getTime()}`}
{...child.props}
className={`k-toolbar-button ${child.props.className ? child.props.className : ''}`}
></Button>
);
} else if (child.type === MenuButton) {
tempToolbarChildren.push(
<MenuButton
key={`${index}-${new Date().getTime()}`}
{...child.props}
className={`k-toolbar-menu-button ${child.props.className ? child.props.className : ''}`}
></MenuButton>
);
} else if (child.type === SplitButton) {
tempToolbarChildren.push(
<SplitButton
key={`${index}-${new Date().getTime()}`}
{...child.props}
className={`k-toolbar-split-button ${child.props.className ? child.props.className : ''}`}
></SplitButton>
);
} else if (child.type === ButtonGroup || child.props.className && child.props.className.includes('k-button-group')) {
const buttonGroupItems: JSX.Element[] = [];
const childrenArray = Array.isArray(child.props.children) ? child.props.children : [ child.props.children ];
childrenArray.forEach((button, bindex) => {
buttonGroupItems.push(
<Button
key={`${bindex}-${new Date().getTime()}`}
{...button.props}
className={`k-toolbar-button ${button.props.className ? button.props.className : ''}`}
></Button>
);
});
tempToolbarChildren.push(
<ButtonGroup
key={`${index}-${new Date().getTime()}`}
{...child.props}
className={`k-toolbar-button-group ${child.props.className ? child.props.className : ''}`}
>
{buttonGroupItems}
</ButtonGroup>
);
} else if (child.type === Combobox) {
tempToolbarChildren.push(
<Combobox
key={`${index}-${new Date().getTime()}`}
{...child.props}
className={`k-toolbar-combobox ${child.props.className ? child.props.className : ''}`}
></Combobox>
);
} else if (child.type === DropdownList) {
tempToolbarChildren.push(
<DropdownList
key={`${index}-${new Date().getTime()}`}
{...child.props}
className={`k-toolbar-dropdownlist ${child.props.className ? child.props.className : ''}`}
></DropdownList>
);
} else if (child.type === ColorPicker) {
tempToolbarChildren.push(
<ColorPicker
key={`${index}-${new Date().getTime()}`}
{...child.props}
className={`k-toolbar-colorpicker ${child.props.className ? child.props.className : ''}`}
></ColorPicker>
);
} else {
tempToolbarChildren.push(child);
}
tempToolbarChildren.forEach(item => {
toolbarChildren.push(item);
});
};
/* eslint-enable complexity */
if (props.children) {
const childrenArray = Array.isArray(props.children) ? props.children : [ props.children ];
childrenArray.forEach((child, index) => {
addUniqueToolClass(child, index);
});
}

I do agree that simplifying the specs is the better approach. I just want to point out that doing so would require us to go through some of the visual tests and update them manually, which would take some time.

Regarding point 4 - I also think that unifying the templates/slots across the different component suites would be a tricky task. The different suites have introduced different templates/slots because of different framework specifics, different users' needs and demand, etc.
I'm not sure if the HTML specs should take care of the templates/slots or if they should take care of the rendering only.
But if so, the HTML specs can provide the guidance of what are the bare minimum of templates/slots that a component should have, the places where a template/slot MUST be implemented.

Regarding point 5 - nothing to add here, apart from what @Juveniel wrote above. By the way, what about the rounded property that we have in several components? It falls in this category as it uses the border-radius utility classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants