Decide on appropriate use of inheritance wrt ancestor component classes

[pospi]

I'm unsure whether strangeness in this inheritance chain is part of the reason there are bugs in component rendering; but it does seem like something we should revisit regardless. Perhaps a mixin-like architecture similar to what ScopedRegistryHost() does could be more idiomatic and composable?

NHComponentShoelace and NHComponentMaterial are currently defined differently and the latter appears not to have its own adapter styles present in any case.

[nick-stebbings]

Yeah, the NHComonentMaterial was sort of a stub in case Wes had any overrides we could use (as I think he is the only one using it currently) but can probably be removed.

Not sure about which bugs you mean with the component rendering. It is working ok in the Dashboard at the mo. What sort of issues have you been encountering?

[adaburrows]

I was thinking about this. It's not good practice to have a component to "be a" non-related thing. But it's more idiomatic for a component to "have a" set of associated styles. For example, in the dashboard, the table now reads to me as being a generic component derived from a base component as opposed to being a table.

Now, if it was named something other than it is which implies it is a table instead of having a table (like NhDashboardResourceView), then maybe I would not have had red flags go off.

I would strongly suggest not having a mix-in or inheritance pattern for styles in the general case for everyone. Rather, there should be plenty of ways for different applications to get at the styles to include in their application. A method I would strongly suggest is just having the variables loaded into the :root css context of the nh-launcher so all the variables are inherited in all the shadow DOM :host contexts. Then we could also have:

• CSS modules (CSS files with relative paths in a JS module, but when imported just expose values as JS objects for use inline)
• CSS built into a JS template literal for use with ShadyCSS, like in Lit (I had originally requested this in the original list of things to do for the design system). This would eliminate the use of

  css`${unsafeCSS(Dark)}`
  

  that we currently have in the code. For an example of this being done, see Shoelace's theme build script. I do like Shoelace's approach to building and organizing things.
• Maybe some other format if we need it.

Then each kind of application can use whichever kind of idiomatic inclusion of CSS they would like. JSX using apps could just import the CSS modules and define the className attribute. Lit apps could just import the correct modules as JS template literals to be passed to ShadyCSS in an array.

If we want to use a mix-in in the dashboard, I'm not against that. But it should be something like:

function DecoratorForLitStyles(...styleList) {
  return function (Class) {
    if (Class.styles && Class.styles instanceof Array) {
      Class.styles = [...styleList, ...Class.styles]
    } else if (Class.styles && Class.styles instanceof Function) {
      Class.styles = () => [...styleList, ...Class.styles()]
    } else if (Class.styles) { // yeah, maybe check if it's a CSSResult, too
      Class.styles = [...styleList, Class.styles]
    } else {
      Class.styles = [...styleList]
    }
  }
}

const WithNhBaseStyles = DecoratorForLitStyles(baseStyles1, baseStyles2)
const WithNhShoelaceAdaptor = DecoratorForLitStyles(shoelaceAdaptorStyles)
// whatever else we need...

Then each component could just do

@WithNhBaseStyles
class NhAssessmentControl extends ScopedRegistryHost(LitElement) {
  static styles = css`
  :host { background-color: #000; }
  :host-context(h1) { border-bottom: 2px dotted white; }
  `
}

Then if components need to inherit from each other the prototype/inheritance chain won't be disrupted. It would also be made available to other people if they want to use our CSS with Lit.

[pospi]

@nick-stebbings

Not sure about which bugs you mean with the component rendering.

Don't worry, that turned out to be a red herring on CustomElement registration stuff.