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

RFC: Theme components, convention, granularity #6116

Open
slorber opened this issue Dec 16, 2021 · 3 comments
Open

RFC: Theme components, convention, granularity #6116

slorber opened this issue Dec 16, 2021 · 3 comments
Labels
apprentice Issues that are good candidates to be handled by a Docusaurus apprentice / trainee proposal This issue is a proposal, usually non-trivial change
Milestone

Comments

@slorber
Copy link
Collaborator

slorber commented Dec 16, 2021

For the 2.0 release (#6113) we want to refactor our theme to make it easier to customize/override/wrap in a robust way

We should aim to reduce due to fragile customizations when upgrading Docusaurus versions.

There are multiple things to consider.

Conventions

We should find a way to organize our theme according to intuitive and easy-to-follow conventions.

We can define a guideline to properly name a component.
For example, one rule could be: if it's implementing the docs plugin, then it must start with Doc

We can define rules for when to nest/not nest components inside a subfolder

We can define naming rules for how to handle lists: should a component be named BlogPostList or BlogPosts?

We can define TS conventions, for example finding a way to ensure all theme components have a Prop type?

Note: it may be better to find retro-compatible conventions. It may not be a good time to rename everything and create a lot of subfolders as it can impact existing users quite badly. We can still do more important refactors later, but having a convention guideline now is still valuable today.

Component granularity

We should split our theme into smaller components.

Creating smaller components permit to avoid copying too much logic when using swizzle (see #6114)

It also gives more convenient ways to wrap an existing component with before/after logic (still see #6114)

At the same time, if we split too much, this becomes complicated to find good component names, and using our current flat structure leads to a quite long/messy folder.

Studying our site showcase customizations (#6115) should help us figure out which components are not granular enough.

In general, I believe we should have at least a dedicated component for each design system element (ie elements from Infima). If an element is used in multiple places (like icons, or pagination arrow buttons), it is not a bad idea to extract it as a separate component, instead of inlining Infima CSS everywhere. We should probably end up with Infima-React design system that we could later add to a Storybook.

It is probably useless to split other components too early: we can wait until someone comes up with a very concrete use-case, and see if it's worth it.

Theme design system

Instead of spreading Infima classes everywhere, we should start to implement our own typesafe theme design system, in React + TypeScript.

Each theme could implement the same design system elements.

We could find an FS structure that clearly separates those design system components from the rest of the theme components.

Each theme (classic, tailwind...) could implement the same design system components with the same interface/props.

We could also use Storybook/Chromatic for DX / visual tests

This would also help to surface some existing design issues in Infima that prevent good CSS encapsulation with a React component model.

Moving technical code to theme-common

We write a lot of helper code that is more related to handling props, global data and things like, that is often quite technical and not really related to the UI/styling aspect of Docusaurus.

Whatever that is not coupled to UI/Infima should rather be moved outside of the classic theme, in theme-common

This should make the user's life easier when overriding theme components, as the amount of copied code can be significantly reduced. We probably can't refactor everything at once but there are probably a few easy-to-move functions and even technical components (not using Infima).

Note that translations might also be a good candidate because if we have a Tailwind theme someday there's no reason that we'll use different labels...

Stable CSS selectors

We should identify what are the most common CSS selector customization needs (#6115)

Each theme component should probably have a root stable selector with an intuitive name (ie probably just the component name?), this should already help a lot making userland CSS selectors more robust.

We should find a way to avoid recommending to users such selectors to target CSS modules: div[class^='announcementBar_']: this is not intuitive, the syntax is hard to remember and this is also quite fragile as 2 CSS module classes might be different but still start by the same prefix.

Always apply className?

It could be useful in various situations to always forward the className prop to the root container of a theme component.

Users could eventually customize a component's CSS without using complex/global selectors, just wrapping it:

import React from "react";
import OriginalFooter from "@theme-original/Footer";

import styles from "./myCustomStyles"

export default function Footer(props) {
  return (
      <OriginalFooter {...props} className={styles.myCustomFooterStyle}/>
  );
}

Just an idea, not sure it's a good one to recommend this pattern 🤷‍♂️ (might even not play well with CSS insertion order?)


These are some non-exhaustive thoughts. Let me know if you have any feedback or other improvements to suggest.

@slorber slorber added the proposal This issue is a proposal, usually non-trivial change label Dec 16, 2021
@slorber slorber added this to the 2.0.0 milestone Dec 16, 2021
@slorber
Copy link
Collaborator Author

slorber commented Dec 20, 2021

Question, as part of #6132 we now have a simple footer and a multi-column footer.

It probably make sense to split this as 2 separate components (I would create a column component, but probably just internally).

I'd find this better to share the same prefix Footer* for all those Footer-related components.
Using FooterSimple, FooterMultiColumn instead of SimpleFooter, MultiColmumnFooter.
Even if the name is less fluid, this is more convenient to understand that files are related.


Another possibility is to nest all those components inside a Footer/ folder. What I don't like with this approach is that we now can't swizzle the Footer component without copying its subfolders too. If we use this approach we should rather never use Footer/index.tsx but instead Footer/FooterRoot/index.tsx

cf what you get when you run yarn workspace website docusaurus swizzle @docusaurus/theme-classic NavbarItem --danger

image

(we could exclude subfolders from swizzle copying, but does it always make sense?)


Whatever we decide, we'd rather be consistent across the whole theme.

The NavbarItem and Footer are probably good use-cases to study.

@Josh-Cena @lex111 , starting from an empty theme, what do you think would be the ideal theme file-system tree for these 2 features?

@Josh-Cena
Copy link
Collaborator

I would try to have a more nested component structure, e.g. Docs/DocItem/DocPaginator rather than a flat DocPaginator. And yes, we should only copy actually relevant components when running swizzle—it's a bug that it currently doesn't and copies nested components.

However, FooterSimple and FooterMulticolumn is unnecessary. We shouldn't split components where only one can possibly be used. If the user decides to swizzle Footer, we have one unique need—it will either by simple or multi-column. We shouldn't need to let the user choose further. The user can also delete unnecessary code if she wants.

Apart from that, I would need to start with actual refactoring draft to determine what's suitable :D It's very hard to make an a-priori one-for-all rule

@Josh-Cena
Copy link
Collaborator

We are going to close #5263 in #6243 because some of the pressing outdatedness will be resolved, but until we figure this out, we can't make the swizzle config the most correct.

@slorber slorber changed the title [2.0] Theme components, convention, granularity RFC: Theme components, convention, granularity Aug 17, 2023
@slorber slorber modified the milestones: 2.0.0, Upcoming Aug 17, 2023
@slorber slorber added the apprentice Issues that are good candidates to be handled by a Docusaurus apprentice / trainee label Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apprentice Issues that are good candidates to be handled by a Docusaurus apprentice / trainee proposal This issue is a proposal, usually non-trivial change
Projects
None yet
Development

No branches or pull requests

2 participants