dojo / widgets

:rocket: Dojo - UI widgets.
https://widgets.dojo.io
Other
88 stars 66 forks source link

Should we provide generic css? #21

Closed rishson closed 6 years ago

rishson commented 7 years ago

Whilst we will provide out-the-box themes for Dojo 2, are we planning on providing generic css, along the lines of:

etc.. I'm thinking in terms of providing non-theme specific classes like this.

Currently, my opinion is that we should provide these css files for importing into widgets. I currently think of Dojo 2 themes as being variations (in terms of colours, corners, gradients etc) on a set of Dojo 2 - wide defaults (the generic css).

rishson commented 7 years ago

@tomdye @itorrey @bitpshr @smhigley - adding you initially for commentary.

rishson commented 7 years ago

An alternative to providing our own generic css, is to have tutorials to show how you can use third party css toolkit with Dojo 2. However, I'm mindful that we should probably also have sensible x-theme defaults.

bitpshr commented 7 years ago

I think the functionality that this CSS (module?) should provide should be very barebones. We don't want to attempt to replace robust CSS toolkits like Bootstrap or anything similar. I sort of air on the side of everyone implementing their own positioning using CSS, but I can see how some utility class could be useful to developers who are unfamiliar with semi-advanced styling techniques.

rishson commented 7 years ago

We will need to provide at a minimum:

so that our own widgets are consistent. I take your point about grids, but then again, our own widgets will be using flexbox for layouts won't they?

Also, whilst robust, bootstrap (upto the unreleased v4) doesn't play well with our stated aim to have top class a11y support in all our widgets (bootstrap still used px rather than em/rem, due to IE8 support). Its a tricky one, and I'm in agreement that supporting a full css toolkit is too much support overhead. Then again, is it more work than providing support for customers who use toolkit 'x' (where we have to work out if the issues raised are internal to Dojo widgets, or in 'x')?

smhigley commented 7 years ago

There are a few utility classes like .visually-hidden and .hidden that I would find useful. I'm not a huge fan of having a lot of global classes or styles, though.

tomdye commented 7 years ago

I don't see the need for the files outlined here, better to keep it simple with the one variables.css that we already have a special rule for. We may have a layout.css at some point that can contain things like pull-right, visually-hidden etc.. as @smhigley said but beyond that I don't see the point. The typography side of things doesn't seem worthy of it's own file as we're only really going to be setting font-family and then font-sizes on a header1, header2 style basis as css-modules steers away from tag names and thus we won't be using them to target styles.

As long as we keep the one variables.css file tidy and well laid out we should not have a problem.

bitpshr commented 7 years ago

I agree with @tomdye and @smhigley, but I don't think we should be setting CSS outside of components. We shouldn't be setting font sizes for the entire page. Just because a user wants to utilize Dojo 2 components doesn't mean they want their entire page to get Dojo 2 default styling.

I still think basic utility classes (as mentioned above) are beneficial. Then a consumer can opt-in to use them if they need them.

rishson commented 7 years ago

OK, so help me see how this would work. Lets say that I want to build a Dojo 2 app and lets say that we think system fonts are a good idea. If variables.css contained:

:root {
    --base-font: -apple-system, BlinkMacSystemFont,
            "Segoe UI", "Roboto", "Oxygen", "Ubuntu", "Cantarell",
            "Fira Sans", "Droid Sans", "Helvetica Neue",
            sans-serif;

now, I would expect an official Dojo 2 theme to include

body {
  font-family: --base-font;
  font-weight: 400;
  letter-spacing: 0;
  font-style: normal;
  text-rendering: optimizeLegibility;
  -webkit-font-smoothing: antialiased;
  -moz-osx-font-smoothing: grayscale;
  -moz-font-feature-settings: "liga" on;
  font-size: 1em;
  line-height: 1.4;
}

so the theme makes all the app look the same font-wise. The user want Dojo 2 to set a page level look and feel and they opt-in by applying our theme i their widgets. My question here, is 'is there any point in putting the --base-font in variables.css at all, or just put in a theme file? We might have a theme that thinks system fonts are awful, or that would rather size in px etc, so I don't see the point of sharing css vars across themes, unless they are truly generic helper classes for our own use, e.g.

--visually-hidden: 
--row:

But to @bitpshr 's point, what happens if the user is only adding a single Dojo 2 widget to an existing app (and doesn't want our theme styling)? Would the widget only use css for structural type stuff and any visual styling (fonts, shadows, borders, padding etc) would have to be applied by an opt-in theme? If so, then we probably need to come up with the guidelines for what widgets should care about. For example, we probably have to state that we use flexbox for layout and our widgets use this comprehensively, we probably have to say 'no visual effects at all in widget css'..

bitpshr commented 7 years ago

Our CSS should provide styling, both structural and theme, but only within components. E.g. all Dojo components should use the same font styling. We shouldn't be messing with the entire page though (so I disagree with the examples above); this was a big complaint with Dojo 1 themes. Dojo is a component framework, so its themes should only effect those components. Simply pulling in Dojo component CSS shouldn't change the entire page's font family, size, etc. But component theme CSS can and should provide more than just structural styling...

tomdye commented 7 years ago

Agree with @bitpshr above, styles should not leak outside of our widgets, one of the reasons for css-modules adoption. On the font-side of things @rishson, I'm leaning towards thinking that we shouldn't set one at all and should adopt whatever has been loaded onto the page perhaps?

bitpshr commented 7 years ago

@tomdye good point about fonts; that's probably a very common aspect of component styling that users will want to change so things blend together nicely. I agree that we shouldn't set one at all inside components and let the page font do its thing.

rishson commented 7 years ago

" and theme, but only within components" - @bitpshr not sure what you mean here. Are you suggesting that we add styles to each widget when we create a new 'official' theme?

"styles should not leak outside of our widgets" - @tomdye , so if we offered a 'material design' theme, this would mean adding sections to every widget's css? I must be misunderstanding, because I can't then see the use of the themeing abilities then..

bitpshr commented 7 years ago

@rishson yes, that's what I'm suggesting. If we're using variables effectively within our components, this task wouldn't be tedious at all.

In your example of a material design theme, how would leaking CSS outside components and applying page-wide styling make this easier? Setting global styles doesn't get us any closer to a material design theme. We'd still have to go and change variables accordingly so components would be styled differently.

I definitely agree with @tomdye about things not leaking outside of components. Our theming should only apply to components, not the page as a whole.

rishson commented 7 years ago

OK, I think I'm closer to understanding, maybe its the contents of a theme file that I'm unsure of. Lets say we don't specify any font props in our variables.css so we take whatever the page gives us. If we were to create a material design compliant theme to apply to all widgets, then where and how would you set the font to Roboto, then default padding to 1em etc.. etc..?

bitpshr commented 7 years ago

Admittedly, I didn't realize material design compliance required such specific font styling. But it shouldn't be a problem:

For default font, each component would use the same --base-font variable (or something similar) that would normally be set to inherit. For a theme that requires stricter defaults, we could just change the value of that variable to whatever we want, such as Roboto. We'd do something similar for default padding (and any other CSS property): components would all use the same variables for common properties, like padding, border-radius, etc. For different themes, we just update that variable and all components change accordingly.

There may still be some edge cases where individual component styling needs to be updated beyond variables, but I can't think of any off-hand.

rishson commented 7 years ago

So, something like:

[variables.css]

  --base-font: inherit;

[material-theme.css]

  --base-font: Roboto;

[MyWidget.css]

  root: {
    font-family: --base-font;
  }

[MyWidget.ts]

v('div', {classes: this.classes('root').get()}

The above meets my understanding of how a theme can override variables, and a widget does not need to change per-theme, but from the README, the theme examples look like they override the classes, e.g. background-color: red; rather than set css variables, so I'm still unsure if this is correct.

dylans commented 7 years ago

I think there are two primary use cases to consider that are at odds with each other which means we're all right some of the time:

So, we should outline the workflow approach for the following scenarios:

bitpshr commented 7 years ago

@rishson that's the gist of what I was thinking, maybe @tomdye can make sure that looks OK.

tomdye commented 7 years ago

@rishson , yes and no, the theme cannot override variables, they don't exist in the built widget css as they've already been computed. It's more a case of:

/* themes/material/variables.css */
:root {
  --base-font: Roboto;
}
/* themes/material/button.css */
@import variables from './variables.css';

.button {
   font: var(--base-font);
}
/* my/project/myWidget.css */
@import variables from '@dojo/themes/material/variables.css';

.myWidget {
   font: var(--base-font);
}
/* myApp.ts */
import Button from '@dojo/widgets/Button';
import myWidget from './MyWidget';

import * as materialTheme from '@dojo/themes/material/theme.css';

// ...
render () {
   return v('div', {}, [
      w(Button, { theme: materialTheme }), // pass in theme
      w(MyWidget, {}) // using theme vars as default
   ]);
}

You have to remember that the built / packaged and delivered css with dijit will already have all variables computed and replaced so any new variables will have zero effect. The theme will have to be set on the themed button and imported into any custom widgets that a user wishes to theme similarly (or they can have a theme set on them the same way as button.

rishson commented 7 years ago

OK - thanks @tomdye . So for this to work:

/* my/project/myWidget.css */
@import variables from '@dojo/themes/material/variables.css

will each Dojo 2 theme have to have values for every variable, used in every Dojo 2 widget or do we somehow mixover @dojo/widgets/styles/variables.css somewhere, so theme css only needs to specify the variables that it overrides? Also, how will we document which variables are used to achieve which effects, so a user knows, for example, that the font-family used in theme X is --base-font?

tomdye commented 7 years ago

No @rishson. You've missed my point, the variables in the basetheme cannot be effected, they will already be converted to values in the baseTheme css that each widget imports, thus there is absolutely zero connection between mytheme/variables.css and @dojo/widgets/styles/variables.css as they effect separate css files.

Each class used in this.classes will be replaced by the theme class if a matching one is found and that is how a theme is applied. Any parts of the underlying basetheme applied using fixed will still be present using the original computed variables.

Just to be clear, we're shipping built css to use with our widgets AND separately shipping our variables.css for custom widget creators / app writers to use in order to match a given theme in their own implementation.

rishson commented 7 years ago

OK - I think I understand it now. My second question still stands though:

Also, how will we document which variables are used to achieve which effects, so a user knows, for example, that the font-family used in theme X is --base-font? Will we have to make sure variable naming is consistent across themes, so that a custom widget creator knows that if they want to replicate the font used in any theme, then the var is always --base-font etc, regardless of which Dojo 2 theme they are replicating?

tomdye commented 7 years ago

it would certainly be worth us having some guidelines for variable naming. I see it more that a client using a theme would tailor their app to that one theme though rather than allowing their app to be fully themeable in itself. In order for their components to be themeable and receive a theme, they would need to extend the themeobject to contain entries for their custom widgets, those entries would use the specific theme variables thus the naming consistencies won't really matter.

rishson commented 7 years ago

Refs dojo/meta#84

eheasley commented 6 years ago

We had decided that providing application level CSS is outside the scope of widgets. We are safe to close this issue out for now.