carbon-design-system / carbon-spec

[WIP] Specification for the Carbon Design System
Apache License 2.0
7 stars 6 forks source link

Component Configuration Objects #8

Open mattrosno opened 5 years ago

mattrosno commented 5 years ago

Continuing discussion from #6...

carbon-spec repo:

  • defines a consistent configuration object structure to be used by framework specific generate functions

In a carbon-components-X repo

  • contains a function that can take a configuration object and output a template context object (generate)

Regardless of what this is called... generate... pre-generate... there's the need for the spec to output configuration objects to help each carbon-components-x build their templates. If we call it a "generate function", we pass in component options (e.g. I want a primary button), and you get back a configuration object. I believe we're really only concerned about three properties here, element, attributes and classes.

Button example:

As carbon-components-x, I want to render a primary button. Spec returns:

{
  "root": {
    "attributes": {
      "type": "button",
      "tabIndex": 0
    },
    "classes": "bx--btn bx--btn--primary",
    "element": "button"
  },
  "content": {
    "classes": "bx--btn__content"
  },
  "icon": {
    "classes": "bx--btn__icon"
  }
}

element and attributes important for a11y, classes important for styling. Everything else can be left to each carbon-components-x.

Button example (as an anchor):

As carbon-components-x, I want to render a primary button that is an anchor tag. Spec returns:

{
  "root": {
    "attributes": {
      "role": "button",
      "tabIndex": 0
    },
    "classes": "bx--btn bx--btn--primary",
    "element": "a"
  },
  "content": {
    "classes": "bx--btn__content"
  },
  "icon": {
    "classes": "bx--btn__icon"
  }
}

root.element and root.attributes.role are updated accordingly, and as long as carbon-components-x properly implements the configuration object from the spec, the button remains accessible as an anchor.

Multiple generate functions

Where button may only need one generate function, other components might need many. Take accordion for example. The primary generate function would tell the component that it needs to be an unordered list with certain classes, but we'd also want a generateItem function for each accordion item to render.

Accordion item (open) example:

As carbon-components-x, I want to render an accordion item that's open. Spec returns:

{
  "item": {
    "classes": "bx--accordion__item accordion__item--active",
    "element": "li"
  },
  "heading": {
    "attributes": {
      "aria-expanded": true,
      "aria-controls": "pane1"
    },
    "classes": "bx--accordion__heading"
  },
  "content": {
    "attributes": {
      "id": "pane1"
    },
    "classes": "bx--accordion__content"
  }
}

Zero to many generate functions that return configuration objects. Those config objects only contain element, attributes and classes for their relevant nodes, and are just enough to inform each carbon-component-x to accessibly render templates, without being over-prescriptive.

elizabethsjudd commented 5 years ago

@mattrosno thanks for setting up this issue. We've done a lot of re-factoring with this concept (it was a big part of our re-architecture we just finished). You're correct that at the end of the day there are 3 things that we care about, the element type, the classes that get applied (which technically could be an attribute), and attributes. The structure you have defined above is what really gets put in to the template but it's not very user friendly which is why we have separated our template object from the configuration object. The configuration object is the easy to define setup to generate the template object. Here's button for example:

config = {
  // this can be set up as a default and is optional
  classes: {
    root: 'my-custom-class',
  },
  variant: 'primary',
  disabled: true,
  icon: 'information' // could be an object as well for more customizations
};

template = {
  // top level properties would be applied ot the root element
  element: 'button',
  classNames: 'my-custom-class bx--btn bx--btn--primary',
  attributes: {
    type: 'button',
    disabled: ''
  },
  // content element (following PAL's structure)
  content: {
    text: 'Button text',
    classNames: '',
    attributes: {},
  },
  icon: {
    classNames: 'bx--btn__icon',
    // how ever we structure icon which is framework specific
  }
}

We have multiple components that export multiple "generate" functions. Take a look at our list, checkbox, and core/table generate files for examples of how we handled this.

mattrosno commented 5 years ago

@elizabethsjudd great. Each component to export zero to many generate functions, where each function takes in an options object (e.g. give me a disabled primary button with an icon), (e.g. give me an accordion item that's expanded).

Regarding what the generate functions return, element, classNames, attributes 👍. What about the structure? In my examples above, each key represents a node, the same keys that we use in the selectors object. Also, would the spec ever modify or provide presentation text? I'm thinking no until we come across that.

With that, your example would read like so:

{
  "root": {
    "element": "button",
    "classNames": "my-custom-class bx--btn bx--btn--primary",
    "attributes": {
      "type": "button",
      "disabled": ""
    }
  },
  "content": {
    "classNames": "bx--btn__content"
  },
  "icon": {
    "classNames": "bx--btn__icon"
  }
}

And carbon-components-x would take it the rest of the way.

As for the icon itself... nested/composite components need to be addressed, but I'm thinking that if the button generate function knows that there's going to be an icon in the button, it sets relevant attributes or classes on button nodes root, content, icon. The button doesn't need to know which icon is being used.

Then, it's up to carbon-components-x to use the icon component spec to generate the actual icon and place it in the button template that is set up expecting there to be an icon.

elizabethsjudd commented 5 years ago

The "output" (what I've been referring to as the template object) of the generate function will be framework specific and will conform to what is needed for a specific framework. It's the "input" (what I've been referring to as the configuration object) of the generate function that we'd like to be consistent across frameworks. That way our users follow the same structure no matter which framework is being used.

We considered doing each node as a key and for shorthand we removed the need for root as every component had this key but I would be fine keeping it how you have above for clarity.

Also, we only provide the default "content" ONLY IF it's required for the component. That way we know that someone is not creating a button without content and allows for quicker prototyping.

As far as nested components, the parent component will have to know to include it and will need to pass along properties to that sub/child component. Button won't care that the "information" icon is being used but developers will need to provide the button that it should use an icon. It's then up to button to make sure that it's children get "generated" as well. You'll see us reference a generators function often in our generate files which is how PAL does it. It determines the child type and then runs the child through it's own generator getting its template object as part of the whole parent's template object

Button is kind of a difficult example of these structures as it's so simple. The portion of the PAL re-architecture for configuration/template objects really focused on consistency across components since we had done a few more complex ones at that point. If you look in our generate.js files the COMPONENT_NAMETemplate (template object) and COMPONENT_NAMEGenerate (configuration object) typdefs are PAL's way of documenting each object's structure for a given component. You'll find a lot of pre-defined (global-typedef) properties that really emphasize how consistent and repetitive elements are.

When you compare the COMPONENT_NAMETemplate and COMPONENT_NAMEGenerate objects, you'll see how we "bubble" up a lot of commonly defined features (variants, modifiers, states, content strings, etc) so that it's easier for the developer to quickly customize a component without having to understand the structure or details of template.

The COMPONENT_NAMEClasses typedef in the selectors.js file show the available elements that are in the template, its keys align with the keys that are in a template object but is more simplified so it's easier to see a high-level view of a component's structure. We've talked about making a different typedef that would just define these keys as you'll see this structure is repeated a lot via our tests, generate, and selectors. At first the COMPONENT_NAMEClasses objects started pretty flat but as we worked on larger/complex components the structure of them started to really mirror the HTML structure:

scottnath commented 5 years ago

don't forget the content

Hey team...just want to make sure when we're talking about configuration objects that the content is always part of that object. Reasoning: many implementations do not need to change any styling and will only want to change the content. This is quadruple-true for folks doing rapid-prototyping.

We should make it dead simple for app developers to configure a component with just a bunch of content. For button that would be:

{
  content: 'click this meow',
}

For a list item that could mean an object like this:

{
  "tag": "ul",
  "items": [
    {
      "children": [
        "Unordered Item 1"
      ],
    },
    {
      "children": [
        "Unordered Item 2"
      ],
    },
    {
      "children": [
        "Unordered Item 3"
      ],
    }
  ]
}

or if we really want to make it easy, and assuming that unordered list is the default when using a list component:

{
  "items": [ "Unordered Item 1", "Unordered Item 2", "Unordered Item 3"]
}

The content may be all many developers care about. I bring this up because @elizabethsjudd you said "You're correct that at the end of the day there are 3 things that we care about, the element type, the classes that get applied (which technically could be an attribute), and attributes."

I posit that for someone doing rapid prototyping, or even an application that wants to use 100% carbon-styles, they wouldn't care about any of configurations around styling/elements/etc and would gladly allow a component's defaults to kick in. They would only care that the content is correct in their prototype.

elizabethsjudd commented 5 years ago

@scottnath you're absolutely right. @mattrosno when you are looking at the typedefs in PAL, you can see that we make short hand ways to defined content. They aren't always called content because there can be more robust structures but that is a big part of why we have configuration objects.

mattrosno commented 5 years ago

@scottnath if the spec is receiving content, and returning the exact same content, what's the point? Is it for simplicity so carbon-components-x has to merge less, and should have everything that it needs from the spec config object to then render the component?

Is it for defaults, in case somebody chooses not to, or forgets to, pass in content and the default content is applicable?

Is it for content modification? I can't think of any example, especially if it's user-generated content, but if we're passing content through the spec, the spec is capable of doing something to the content via generate functions in the future?

Content of course is important when using a component. Just making sure that it needs to pass through the spec.

scottnath commented 5 years ago

The spec doesn't receive or send content (except for the default content used by a given demo). The spec defines the configuration and contains tests which ensure that configuration is accepted by c-c-frameworkx. The spec tests that framework-x's output creates the HTML and VRT snapshots that are expected when it receives the configuration object for a given component.

So my comment is in relation to the definition of the configuration object. I'm just making sure that when the object for a given component is defined, that a content (or equivalent) property is part of that definition for each element within a component.

ie: If our working theory is that each c-c-frameworkx should accept the carbon-spec configuration object for a given component, then we should make sure that the content for each element is part of the component configuration object specification.

joshblack commented 5 years ago

I believe we're really only concerned about three properties here, element, attributes and classes.

Still trying to catch up on spec stuff, could you help me understand why one would leverage elements/classes in this object? Initial thoughts in this space would be that component APIs encapsulate the content/domain model of the component and not accepting elements/classes as those are implementation details (that should be defined as a rule in the spec).

When thinking about this in a test runner setup, I would imagine something like:

/* React.js framework test file */

// Proposed API
import { run } from '@carbon/spec';
import React from 'react';
import ReactDOM from 'react-dom';
import { ComponentName } from '../entrypoint';

describe('ComponentName', () => {
  let mountNode;

  beforeEach(() => {
    mountNode = document.createElement('div');
    document.body.appendChild(mountNode);
  });

  afterEach(() => {
    mountNode.parentNode.removeChild(mountNode);
  });

  it('should follow the carbon spec', async () => {
    const runner = run('ComponentName');

    // Similar to `mount` function, provide the node you rendered the
    // component to
    runner.beforeEach(descriptor => {
      // descriptor provides content model for initialization
      const { title, children } = descriptor;
      ReactDOM.render(
        <ComponentName title={title}>{mapToChildren(children)}</ComponentName>,
        mountNode
      );
      return mountNode;
    });

    // Could be async if we want to parallelize tests
    await runner.run();
  });
});
elizabethsjudd commented 5 years ago

could you help me understand why one would leverage elements/classes in this object?

I don't think that the spec repo would necessarily "leverage" these properties. It's more of a documentation of what the configuration object should be so that way when the framework specific implementations are built to generate a template object the input would be consistent across the frameworks.

scottnath commented 5 years ago

With the end goal being symmetry between the frameworks, that should include the elements because final HTML should match exactly. The elements need to be known also because they are defined within the selectors object, which contains what is essentially mapping of classNames to selectors (elements).

For your example, I assume that classNames may be applied in the entrypoint file. One goal of a component's configuration object (api) would be to allow an app developer to inject their own classNames on top of the default carbon ones. So looking at this:

<ComponentName title={title}>{mapToChildren(children)}</ComponentName>

regardless whether it's inside or outside a test runner, having a consistent api would mean one could add classnames: <ComponentName title={title} classes={appSpecificClasses}>{mapToChildren(children)}</ComponentName> in the same way they might with the handlebar's generate function const config = generate({classes: appSpecificClasses})

I agree that a component shouldn't receive an actual element declaration - like we wouldn't want button to be what defines a <button like <config.element, but we do need the config object to know the mapping - which is covered by the selectors object.

joshblack commented 5 years ago

It's more of a documentation of what the configuration object should be so that way when the framework specific implementations are built to generate a template object the input would be consistent across the frameworks.

@elizabethsjudd Gotcha, could you share an example of how a component might be built using this in something like React or Vue?


One goal of a component's configuration object (api) would be to allow an app developer to inject their own classNames on top of the default carbon ones.

@scottnath this feels like it might be overlapping with the frameworks here. I definitely agree that part of a component API is that you can supply a custom class string, but I think that's a developer affordance versus a specification requirement. I also don't quite understand how the generate function works with respect to calling a component. As an app developer, I would assume the following API for supplying a custom class name:

<ComponentName title={title} className="custom-classname">
  <ChildA />
  <ChildB />
  <ChildC />
</ComponentName>

Is the idea that we would instead do:

import { generate } from '@carbon/spec/components/name';

const props = generate({
  // options
  class: 'custom-class',
});

<ComponentName {...props}>
  <ChildA />
  <ChildB />
  <ChildC />
</ComponentName>

?

mattrosno commented 5 years ago

Custom class names - let's say you want to render a React button:

<Button content="My Button" className="some-class" />

In the component API, you would:

const button = buttonConfig('bx');
const myButton = button.generate({
  disabled: true, // TODO get from props
  size: 'small', // TODO get from props
  variant: 'danger', // TODO get from props
  content: 'My Button' // TODO get from props
});

And the spec returns this configuration object:

{
  "root": {
    "attributes": {
      "type": "button",
      "disabled": ""
    },
    "classNames": "bx--btn bx--btn--danger bx--btn--sm",
    "element": "button",
    "content": "Button"
  },
  "content": {
    "attributes": {},
    "classNames": "bx--btn__content"
  },
  "icon": {
    "attributes": {
      "aria-hidden": true
    },
    "classNames": "bx--btn__icon"
  }
}

Which can get used to render() the button.

Either the component API takes props.className ("some-class") and concatenates with root.classNames, or we pass "some-class" through the spec and the spec does the string concatenation to support component styling customizations.

If part of spec is verifying that component APIs support custom class names, and if providing component config objects through the spec, then ya, it makes sense to pass the custom class names through the spec.

const button = buttonConfig('bx');
const myButton = button.generate({
  disabled: true,
  size: 'small',
  variant: 'danger',
  content: 'My Button',
  classes: {
    root: 'some-class',
    icon: 'another-class',
  },
});

Returning:

{
  "root": {
    "attributes": {
      "type": "button",
      "disabled": ""
    },
    "classNames": "some-class bx--btn bx--btn--danger bx--btn--sm",
    "element": "button",
    "content": "My Button"
  },
  "content": {
    "attributes": {},
    "classNames": "bx--btn__content"
  },
  "icon": {
    "attributes": {
      "aria-hidden": true
    },
    "classNames": "another-class bx--btn__icon"
  }
}
joshblack commented 5 years ago

This feels like it could be a good amount of work to maintain for the spec. Could we simplify this process so that it initially is doing verification testing? I am anxious about working with this pattern in something like DataTable because of its hierarchy. With complex components like that, it would seem like we would have to do one of the following patterns:

If we take the getAttributes helper from icon-helpers https://github.com/IBM/carbon-elements/tree/master/packages/icon-helpers I think we'd find a very similar pattern where we are trying to render the attributes consistently across the packages.

Even though this helper is only responsible for a subset of attributes, we do end up with differences for things like style between the frameworks that one would need to write additional code for as a framework author. Thankfully in elements we are able to generate components at build time and this allows us to pre-compute some of these conversion requirements before runtime.

If the spec is already running rulesets that are looking for what is in a configuration object, do we need a runtime strategy to use it in frameworks or could we just defer to the framework maintainers to make the right choice for them as long as it follows the spec?

elizabethsjudd commented 5 years ago

@joshblack We've been talking about changing the definition of the POC that Matt is currently working on. I'm in agreement that there is some of this work does not belong in this repo. Scott and i were trying to get a consistent "input" terminology which could be part of the spec but how/what the framework does with that input is specific to the framework.