Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Save bradenmacdonald/34bf8bfc8f737f9b23a2664091b8f115 to your computer and use it in GitHub Desktop.
Save bradenmacdonald/34bf8bfc8f737f9b23a2664091b8f115 to your computer and use it in GitHub Desktop.

React guidelines to support content theming in Open edX (Braden's proposal)

  1. Build the UI out of small, modular React components as much as possible.
  2. Build two types of components: "customizable" ones that only compose others using JSX and contain little-or-no HTML nor logic, as well as "internal" components that contain logic and detailed HTML and CSS, etc.
  3. In customizable components, include placeholders like {this.extraContent} in the render() method so that subclasses don't have to override render().

Bad example:

class Header extends React.PureComponent<Props, State> {
    public render() {
        return (
            <header>
                <div className="logoArea">
                    <img src={this.props.brandedLogo} alt={this.props.siteName} />
                </div>
                <div className="mainNav">
                    <ul>
                        <li>
                            <a href="/">Home</a>
                            {this.props.isLoggedIn ? null : <a href="/login">Login</a>}
                        </li>
                    </ul>
                </div>
                <div class="currentUserAvatar">
                    <!-- User avatar, account menu etc. -->
                </div>
            </header>
        );
    }
}

Good example:

// Customizable Header
class _Header extends React.PureComponent<Props, State> {
    public render() {
        return (
            <header>
                <SiteLogo/>
                <MainNav/>
                <UserAvatar/>
            </header>
        );
    }
}
// Customizable Main Navigation Area
class _MainNav extends React.PureComponent<Props> {
    public render() {
        return (
            <MainNavWrapper>
                <a href="/">Home</a>
                <LoginLink/>
                {this.extraNavLinks}
            </MainNavWrapper>
        );
    }
    get extraNavLinks(): JSX.Element[] { return []; }
}
// Internal MainNavWrapper - not meant to be modified in most cases)
class _MainNavWrapper extends React.PureComponent<Props> {
    public render() {
        return <div className="mainNav">
            <ul>
                {React.Children.map(this.props.children, (child) => (child ? <li>{child}</li> : null))}
            </ul>
        </div>;
    }
}

// Default Theme:
export const Header = _Header;
export const MainNav = _MainNav;
export const MainNavWrapper = _MainNavWrapper;

And here's an example of a custom theme:

class MyThemedHeader extends _Header {
    public render() {
        return (
            <header>
                {/* Replace <SiteLogo/> with a fancy widget */}
                <MyCustomAnimatedLogoWidget/>
                <MainNav/>
                <UserAvatar/>
            </header>
        );
    }
}
// Customizable Main Navigation Area
class MyThemedNav extends _MainNav {
    get extraNavLinks() {
        return [
            <a href="/about">About Us</a>,
        ];
    }
}

// My theme:
export const Header = MyThemedHeader;
export const MainNav = MyThemedNav
export const MainNavWrapper = _MainNavWrapper;

I've left out all the Redux glue code, etc., but you can get the idea.

@mtyaka
Copy link

mtyaka commented Jun 20, 2018

Nice. We should also think about data/props that each component has access to. When creating a custom Header, I might want to access some data from the global redux store which the original Header does not access/need. With redux all components usually have access to the store, so I guess that part is covered.

I'd like to see a global redux store which all components have access to. The layout of the data in the global store should be treated as an API (breaking changes announced). The global store should be pre-filled with some common data (current user, current course, etc), while allowing to fetch less commonly needed data from REST APIs using built-in redux actions.

Comprehensive theming quickly becomes hard to maintain when overriding a lot of templates and the main reason is that it's hard to keep track of which template variables were added, removed or changed between edX releases. We could easily end up with the same problem with React components if we don't have a plan for this. I propose a global redux store that each component has access to. The redux store should allow access to all data available to the system. The data relevant to the current page could be hydrated from the template when constructing the component, but any component should still be able to fetch unrelated data via methods on the global redux store, which would in turn fetch the data from an API (either REST or GarphQL). Use case: I want to display the number of all active courses in the system in my custom Header, even though the stock Header component does not have/need this information.

@bradenmacdonald
Copy link
Author

@mtyaka I didn't get a notification about your comment ^ for some reason, but I agree. Thanks for the additional ideas.

@clemente
Copy link

@bradenmacdonald @mtyaka Is it possible to separate components in customizable and non-customizable?
Isn't it always possible to redefine any part of any component, including its logic? For instance, to change what happens on click, how to order the data, etc.
In that case, the proposed system could be framed as „a way to extend the platform by redefining components“, and it would be more powerful or interesting.
(If we could make the concept of „extension“ include some React code and some Python code, then we could register in an ansible variable a list of small extensions, where each extension can add or change Python code, frontend logic, frontend content, and design (as required).)

Is separating logic and style practical, or would it require to redefine components twice (an internal one, and then its customizable version)? Also, if the separation of functions can't be enforced, we're back at the scenario where a theme can redefine all logic (which already happens with mako templates).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment