Better User Experience when Using Themes

[kitsonk]

Enhancement

When creating a new theme, it can be rather rather confusing trying to figure out exactly why classes are not being swapped out for the themed classes. Currently if the themeable mixin can't resolve a class name when given a theme, it silently fails.

An enhancement would be to log a warn to the console when a themeable widget has a theme in the properties, but the mixin cannot resolve the them. Something like:

Cannot resolve themed class name.  Looking for 'root' in 'dojo-mywidget'.
Cannot resolve themed class name.  Missing key of 'mycssmodule'.

These would help the developer know where they may have omitted or misconfigured something.

[tomdye]

It should already output a console message in the event of not finding a matching class name and suggest using fixed instead. It's likely worth making this message clearer though if possible

[kitsonk]

Just to check I was sane, I just tried it. I had the following in a theme:

import * as editor from './editor.m.css';
import * as runner from './runner.m.css';
import * as scrollbar from './scrollbar.m.css';
import * as tabPane from './tabPane.m.css';
import * as treePane from './treepane.m.css';

export default {
	'dojo-editor': editor,
	'dojo-runner': runner,
	'dojo-scrollbar': scrollbar,
	'dojo-tabPane': tabPane,
	'dojo-treepane': treePane
};

And then dropped out dojo-editor. The classes reverted to original ones and nothing was logged to the console.

I also edited the editor.m.css and changed the .root class to .base (which is incorrect). Again, the class applied was the original and nothing was logged to the console.

Under what conditions do things actually get logged? I think in both the above cases something should be logged.

[tomdye]

Dropping out dojo-editor and receiving no logs is the correct functionality as far as I'm aware, as well as removing root. Themes can be partial and thus there's no requirement for all classes or even all widgets to be themed.

[kitsonk]

I disagree. When you expressly apply a theme to a widget, it would be good to know when the classes are not found. Especially if there is no styling in the base CSS file and any styles that are not intended to be themeable are fixed.

[tomdye]

But your theme might only provide a single class within the widget it's targeting even though it has more themeable classes than that. That is not an error.

[kitsonk]

When a themeable mixin tries to resolve a class name in the object that is assigned to the this.properties.theme and does not find it, it is an error (or at least a warning). The developer clearly intended to theme the classes of that widget by setting the property theme on the widget.

What is the use case you mention above where something gets logged? Because if the above is not what you consider an error, what is?

[kitsonk]

Thinking about this use case more and seeing some of the real-world challenges that are occurring, my opinion on what should occur at runtime is:

• If a theme is passed or injected into a widget, and a class is specified as a "non-fixed" class, and the theming system cannot resolve that class named, it should warn with something like:

  Cannot resolve themed class name "root" in key "dojo-mymodule". (MyWidget)
  Cannot resolve themed key "mycssmodule". (MyWidget)
  

• If there is no theme passed or injected, then this silently fails as it does today.

• We might consider only emitting this warnings if running in debug mode (see Standardise on has('debug') for debugging/testing features core#174), but I think with the agreement that base themeable styles are minimalistic, and fixed styles are the structural styles, if a widget cannot resolve a class name when a theme is assert, this is an error (or at least a warning)