Docs: extension styling and theming #1267

aleksfront · 2020-11-06T13:24:54Z

Signed-off-by: Alex Andreev alex.andreev.email@gmail.com

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

stefcameron

This is a great start at documentation for styles! It'll be very helpful, I'm sure. I learned about flex.box thanks to this; I knew there was some kind of library being used for flexbox in Lens, but didn't know what it was. Now at least I know what some styles available to me are.

My feedback in comments...

stefcameron · 2020-11-06T16:20:02Z

docs/extensions/capabilities/styling.md

@@ -0,0 +1,104 @@
+# Styling
+Lens provides a set of global styles and UI components that can be used by any extension to preserve look and feel of the application. However, it’s always up to the developer whether to use them or provide completely different styles for the extension.


I think you can only make this claim

However, it’s always up to the developer whether to use them or provide completely different styles for the extension.
if you have fixed #1251

Removed for now

stefcameron · 2020-11-06T16:43:52Z

docs/extensions/capabilities/styling.md

+If you want to follow a selected theme to keep the 'native' Lens look and feel, respecting the light/dark appearance of your extension, you can use provided variables and build-in lens components such as buttons, dropdowns, checkboxes etc.
+
+## Injected styles
+Every extention is affected by list of default global styles defined in `src/renderer/components/app.scss`. These are basic browser resets like setting `box-sizing` property for every element, default text and background colors, default font size, basic headings visualisation etc.


This is the stuff I think extensions should be isolated from (including flex.box as mentioned earlier) as part of #1251. Overriding these to reset them would be quite a challenge. I realize that's separate from this issue.

For now, this paragraph is very useful, since it states the current behavior, and points users to the Lens code that defines all the styles.

stefcameron · 2020-11-06T18:07:31Z

docs/extensions/capabilities/styling.md

+### vars.scss
+In `src/renderer/components/vars.scss` there are list of predefined variables mostly for dimensions and fonts.
+```
+// Dimensions
+$unit: 8px;
+$padding: $unit;
+$margin: $unit;
+$radius: ceil($unit * .3);
+
+// Fonts
+$font-main: 'Roboto', 'Helvetica', 'Arial', sans-serif !default;
+$font-monospace: Lucida Console, Monaco, Consolas, monospace;
+$font-size-small: floor(1.5 * $unit);
+$font-size: floor(1.75 * $unit);
+$font-size-big: floor(2 * $unit);
+$font-weight-thin: 300;
+$font-weight-normal: 400;
+$font-weight-bold: 500;
+...
+```


This is good to know. #1251 should move all these into CSS variables to make them more accessible, like the colors are, regardless of using SCSS or not.

I'm also not sure how an extension would reference this SCSS file. It's not published in the @k8slens/extensions package...

Having the file reference is useful, but without being able to reference it in the extension's SCSS build, not very useful at all. If they were CSS variables, that would be a different story...

Converted all of these into CSS vars.

stefcameron · 2020-11-06T18:09:24Z

docs/extensions/capabilities/styling.md

+### theme-vars.scss
+In `src/renderer/themes/theme-vars.scss` there are list of theme-defined colors. Most of their values are different for light and dark themes. You can use them to preserve consitent view of extension with respect of selected theme.


Same here: How would I reference this in an extension's build? These all reference CSS vars, so that's much better (more accessible to a wider range of extension implementations; my extension doesn't use SCSS, BTW, it uses Emotion and Lens components).

If you want to encourage extension developers to use SCSS, then you need:

to provide these SCSS files via some NPM package import;

document what all these variables are and where they should be used;

make sure that all your extension examples come with SCSS processor "built-in" to the build (none of them do, right now).

I guess there's no urge for now to encourage developers to use exactly SCSS since the only benefit currently is to use few basic variables. I've transfered them into CSS vars and removed encouragement message.

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

aleksfront · 2020-11-09T11:29:21Z

@stefcameron Thanks for your detailed feedback! I've made changes and added Color reference page where each theme color is documented.

ixrock

What if we make theming files as pure css-files instead of json?
Like this:

/**
  @file theme/lens-some-theme.css
  @name Lens Dark
  @type dark
  @description Original Lens dark theme
  @author Mirantis
*/

:root {
  --mainBackground: green;
  // ...
}

Pros:

easier to navigate in project + refactor when needed
easier to apply theme (turn on/off style element)
all colors preview are shown in editor when file is opened (e.g. vscode, webstorm)
comments inside .css are allowed

Cons:

to show theme's metadata we would need to parse jsdoc (or similar) top comment

docs/extensions/capabilities/styling.md

ixrock · 2020-11-09T14:47:02Z

src/renderer/components/vars.scss

+$font-main: var(--font-main);
+$font-monospace: var(--font-monospace);
+$font-size-small: var(--font-size-small);
+$font-size: var(--font-size);
+$font-size-big: var(--font-size-big);
+$font-weight-thin: var(--font-weight-thin);
+$font-weight-normal: var(--font-weight-normal);
+$font-weight-bold: var(--font-weight-bold);


Do we still need all these scss-var -> css-var aliases?

I think so, since the values were moved into CSS Vars -- unless you SCSS code references the CSS Vars directly, but if that were the case, then I would imagine there would be many more files updated in this PR to move away from using the SCSS variable names.

stefcameron

Nice improvements! Having that color reference, as well as the layout and font variable references, will be a huge help to developers!

docs/extensions/capabilities/color-reference.md

stefcameron · 2020-11-09T15:44:26Z

docs/extensions/capabilities/styling.md


 ## Styling approach
-Lens heavily uses SCSS preprocessor and it's advised to stick with same approach for extension developers in order to use some of the predefined variables and mixins described below.
+Lens heavily uses SCSS preprocessor with a set of predefined variables and mixins.


Since you've now moved everything pertinent to extensions into CSS variables, I would recommend removing this line. Extensions don't need to care about how Lens does things internally. They should only care about how they're supposed to do things, and what Lens does that will affect them (namely, the CSS variables available to them, as well as anything that would affect layout like flex.box, which you mention below).

stefcameron · 2020-11-09T15:51:06Z

src/renderer/components/vars.scss

+$font-main: var(--font-main);
+$font-monospace: var(--font-monospace);
+$font-size-small: var(--font-size-small);
+$font-size: var(--font-size);
+$font-size-big: var(--font-size-big);
+$font-weight-thin: var(--font-weight-thin);
+$font-weight-normal: var(--font-weight-normal);
+$font-weight-bold: var(--font-weight-bold);


I think so, since the values were moved into CSS Vars -- unless you SCSS code references the CSS Vars directly, but if that were the case, then I would imagine there would be many more files updated in this PR to move away from using the SCSS variable names.

docs/extensions/capabilities/styling.md

stefcameron · 2020-11-09T16:27:26Z

@ixrock

What if we make theming files as pure css-files instead of json?

This is something to consider. It would add one less step of indirection between styles and their definition. But note that there are JSON parsers out there that support comments, so you could still add comments in your JSON file, albeit that may be difficult depending on what is parsing that JSON file (if you can customize the parser it uses).

JSON or CSS, however, I think the best thing here would be to colocate the definition of the variables with the variables themselves. IMO, the closer the docs are to the code, the more likely they are to remain accurate. So if we could take all those color variable explanations in color-reference.md and move them into JSON or CSS comments next to each variable they apply to, and then generate color-reference.md from this code, that would be a real win for everyone.

On a related note, since Lens is using CSS variables, this article might be interesting, particularly how Kent implements theme switching with <body data-theme="light | dark"> and a matching CSS selector that defines the same set of variables, but with different colors. https://epicreact.dev/css-variables/

jakolehm · 2020-11-09T17:06:02Z

Note to reviewers: let's improve things in follow-up PRs 🙂

Building on lensapp#1267 and iterating on the docs, re-organizing them into Layout and Theme sections. Also adding sample code on how to detect theme changes in JavaScript.

Building on lensapp#1267 and iterating on the docs, re-organizing them into Layout and Theme sections. Also adding sample code on how to detect theme changes in JavaScript. Signed-off-by: Stefan Cameron <stefancameron@SC-MBPt13-2018.austin.rr.com>

Building on #1267 and iterating on the docs, re-organizing them into Layout and Theme sections. Also adding sample code on how to detect theme changes in JavaScript. Signed-off-by: Stefan Cameron <stefancameron@SC-MBPt13-2018.austin.rr.com>

Initial styling docs

8e3223d

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

github-actions bot added the area/documentation label Nov 6, 2020

stefcameron reviewed Nov 6, 2020

View reviewed changes

aleksfront added 3 commits November 9, 2020 13:33

Adding more details to styling section

26b2254

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

Adding color reference page

c4cfc64

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

Convert basic scss vars to css vars

35fec8e

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

aleksfront marked this pull request as ready for review November 9, 2020 10:39

aleksfront requested a review from a team November 9, 2020 10:39

Fixing urls

4b4808e

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

jakolehm requested a review from ixrock November 9, 2020 10:47

Adding lists for color reference

839b012

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

ixrock approved these changes Nov 9, 2020

View reviewed changes

stefcameron reviewed Nov 9, 2020

View reviewed changes

jakolehm merged commit 8d1c29a into master Nov 9, 2020

jakolehm deleted the docs/extension-styling branch November 9, 2020 17:05

aleksfront mentioned this pull request Nov 10, 2020

Improve lens theming #1298

Open

jakolehm mentioned this pull request Nov 10, 2020

Release v4.0.0-alpha.5 #1311

Merged

stefcameron mentioned this pull request Nov 11, 2020

Update styling docs and add example on theme switch detection #1339

Merged

aleksfront mentioned this pull request Nov 13, 2020

Docs: theming and styling an extension #1257

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docs: extension styling and theming #1267

Docs: extension styling and theming #1267

aleksfront commented Nov 6, 2020 •

edited

Loading

stefcameron left a comment

stefcameron Nov 6, 2020

aleksfront Nov 9, 2020

stefcameron Nov 6, 2020

stefcameron Nov 6, 2020

aleksfront Nov 9, 2020

stefcameron Nov 6, 2020

stefcameron Nov 6, 2020

aleksfront Nov 9, 2020

aleksfront commented Nov 9, 2020

ixrock left a comment •

edited

Loading

ixrock Nov 9, 2020

stefcameron Nov 9, 2020

stefcameron left a comment

stefcameron Nov 9, 2020

stefcameron Nov 9, 2020

stefcameron commented Nov 9, 2020

jakolehm commented Nov 9, 2020

		@@ -0,0 +1,104 @@
		# Styling
		Lens provides a set of global styles and UI components that can be used by any extension to preserve look and feel of the application. However, it’s always up to the developer whether to use them or provide completely different styles for the extension.

		### theme-vars.scss
		In `src/renderer/themes/theme-vars.scss` there are list of theme-defined colors. Most of their values are different for light and dark themes. You can use them to preserve consitent view of extension with respect of selected theme.

Docs: extension styling and theming #1267

Docs: extension styling and theming #1267

Conversation

aleksfront commented Nov 6, 2020 • edited Loading

stefcameron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aleksfront commented Nov 9, 2020

ixrock left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefcameron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefcameron commented Nov 9, 2020

jakolehm commented Nov 9, 2020

aleksfront commented Nov 6, 2020 •

edited

Loading

ixrock left a comment •

edited

Loading