Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Theme architecture #104

Closed
oleq opened this issue Jan 27, 2016 · 8 comments
Closed

Theme architecture #104

oleq opened this issue Jan 27, 2016 · 8 comments

Comments

@oleq
Copy link
Member

oleq commented Jan 27, 2016

Idea

The idea is that CSS in CKEditor 5 will come from three separate sources:

  1. Base theme
    image
    • Developed by CKEditor team.
    • Supports all standard components from the UI Library.
    • A separate project.
    • Similar to Drupal's Classy.
  2. Default theme
    image
    • Developed by CKEditor team.
    • Supports all standard components from the UI Library.
    • A separate project.
  3. Component theme
    image
    • Provided by component developer.
    • Supports a particular UI component.
    • Delivered with a component.

Implementation

  • The builder will discover, concatenate and build .less files into a single CSS file.
  • Source map must be generated so there's a 1:1 relation between concatenated less file and output CSS file.
  • Target CSS file must be uncompressed in development environment.
  • (research needed) Builder must know which less files are to be used:
    • JS–level discovery: don't use less files of UI Components which are not imported.
    • Static code analysis: use tags to indicate which core less files are necessary to build a 3rd party component.
      • Related issue Feature: Introduced PendingActions plugin. ckeditor5-core#127
      • Case: A 3rd party component, which uses variables and mixins from core button.less, but its JS does not refer to button.js. In fact, button.js may not be used at all, so the button.less. A 3rd party component would need to explicitly inform the builder that it needs core button.less.
    • Dependencies stored in custom tags within package.json.
  • (research needed) Less files architecture, which allows importing variables and mixins without rendering the actual file
    • Case: A 3rd party component uses variables and mixins of some core component but the core component itself is not used in the bundle (possibly an edge case).
  • (research needed) How to handle static resources like images, fonts, etc?

Related issues

@fredck
Copy link
Contributor

fredck commented Jan 27, 2016

That's a very good start, @oleq. Well done!

I would be very focused on creating some "base abstract components" to support (kinda) inheritance from the main components. Maybe that's what you meant with "base-box". Just want to be sure that we'll have things like "base-box" (or "inline-box"), "container-box", "floating-box", etc. This means that almost all components will inherit from one of the "base" ones.

Then when it comes to naming, will the ".base" classes endup in the final CSS? Hopefully not as they're not really useful. Otherwise we'll need to prefix them with ".cke" as well.

It's not specified that custom themes will be able to inherit from other themes (specially the "default" one). I would make it clear because this will be one of the easiest ways to create custom themes, based on existing ones (overriding rules, for example). It would be great to understand how optimized the final CSS would look in this case, for example whether overiden rules will not get there twice (the original and the override).

@Reinmar
Copy link
Member

Reinmar commented Jan 27, 2016

Some minor and OT things:

  1. ckeditor5-plugin-customcomponent – we don't use the plugin name in this sense any more. We can actually think about a prefix for ui components (ui-?).

  2. All .less files should be in theme/ directories. I'm not sure that you made it intentionally but it looks like the ckeditor5-theme-* packages does not contain this directory. It's similar to the src/, dev/ and tests/ directories – no code should be placed outside them.

  3. This:

    The builder will discover, concatenate and build .less files into a single CSS file.

    We didn't discuss this yet, but for clarity we may talk about builder and bundler separately. Builder discovers, transpiles and copies files to dist/. Bundler concatenates them. These are separate steps. Building is needed in all workflows, while bundling only when going to production.

    However, there's one reason why these processes can't be totally split – source maps. To create source maps for a bundle you may need to take LESS files, concatenate them and transpile. While in the dev workflow you will transpile each file separately because (I presume) we want to be using multiple CSS files for development. On the other hand, it might be easier if we had always used the first process, because we wouldn't need an automatic CSS loader for components [1]. On the third hand, it does not seem to be hard to implement automatic CSS loader.

  4. As for "Builder must know which less files are to be used" I think that if UI components (JS classes) where pointing to files which they require, we could use this knowledge to discover which LESS files are needed. The only case which this does not solve automatically is the one you mentioned. So a second layer of imports must be used and that's simply LESS imports. In your example, that custom component's LESS file would import button's LESS file. It of course must also list the button's package in its package.json -> dependencies.

Other than that, big +1.

[1] A long time ago we discussed that CKEditor should not be loading its CSS automatically because it's super irritating when your website uses an asset pipeline. We agreed that there will be nothing bad in requiring that a developer includes a CSS file manually. However, it can work this way in the production mode, but in dev mode we can have an automated loader or an index file which will import all CSS files which the builder produced.

One more thing – I favour multiple CSS files in the dev workflow because the watcher (builder) does not have to rewrite so much code every time some source file has changed.

@oleq oleq mentioned this issue Jan 27, 2016
5 tasks
@SteveTheTechie
Copy link

[1] A long time ago we discussed that CKEditor should not be loading its CSS automatically because it's super irritating when your website uses an asset pipeline. We agreed that there will be nothing bad in requiring that a developer includes a CSS file manually. However, it can work this way in the production mode, but in dev mode we can have an automated loader or an index file which will import all CSS files which the builder produced.

Could you maybe give the end developer the option in the production build? Perhaps it could be as simple as optionally including a "plugin" (or the CKE5 equivalent) that auto-loads the CSS.

@SteveTheTechie
Copy link

BTW, I know you avoid any jQuery dependencies, but I really like the jQuery UI Theme Roller approach. I like being able to build my own theme that integrates well stylistically with my web app. A default theme is useful and necessary, but allowing simple generation (via a separate builder web app) of new themes would be huge help.

@oleq
Copy link
Member Author

oleq commented Jan 28, 2016

BTW, I know you avoid any jQuery dependencies, but I really like the jQuery UI Theme Roller approach.

Yes, CKE5 will not depend on jQuery but I got you point about theme customisation. I know only too well how hard could it be. And I think the answer, though not as pretty and intuitive as Theme Roller, will be the CSS preprocessor. With just a few changes to the variables, developers will be able to change the main colors, fonts, paddings, etc. of the entire UI. And given that Chrome is capable of reloading CSS live from, let's say, Less, it gets quite easy for people to customise the editor.

@Reinmar
Copy link
Member

Reinmar commented Jan 28, 2016

Could you maybe give the end developer the option in the production build? Perhaps it could be as simple as optionally including a "plugin" (or the CKE5 equivalent) that auto-loads the CSS.

Hm... sounds interesting. This may be the best solution.

@fredck
Copy link
Contributor

fredck commented Jan 28, 2016

Could you maybe give the end developer the option in the production build? Perhaps it could be as simple as optionally including a "plugin" (or the CKE5 equivalent) that auto-loads the CSS.

+1

@Reinmar
Copy link
Member

Reinmar commented Apr 20, 2018

Cleaning up old discussions. See #186.

@Reinmar Reinmar closed this as completed Apr 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants