Customization patterns

[agjohnson]

From a discussion in #47

This is the beginning of a discussion of some customization patterns that we might provide theme maintainers and authors customizing the look and feel of their own documentation.

Some top level points:

• Theme customization likely includes disabling some of our elements that we're injecting
• Visual customization needs could be quite minimal and our CSS variables are enough

Here are a few patterns:

Element template customization by JavaScript

Most obvious would be allowing customization of the class used for our element:

class MyCustomNotificationElement extends NotificationElement {
  render() {
    return html`
      <div class="some-bootstrap-classes">
        <h1>${title}</h1>
        <p>${body}</p>
      </div>
    `
  }
}

// Pseudo library usage, this pattern doesn't exist yet
readthedocs.NotificationAddon.elementClass = MyCustomNotificationElement

But this is heavy on the JS side too, of course.

Element template customization by HTML script

Something like <script type="text/html">...</script> is really approachable.

There are some injection attack worries here to work through (see the Lit unsafeHtml tag/function for some background). This might influence this pattern.

Element template customization by inner HTML

Not sure this is an option even, and it likely has the same trouble with unsafe HTML, but:

<readthedocs-notification>
  <div class="bootstrap-classes">
    ${title}
    <p>${body}</p>
  </div>
</readthedocs-notification>

More granular Lit elements

I'm not even entirely sure on this one. It feels like JS API consumption would be easier.

But, for example, a Sphinx theme want something like this instead of a notification:

<div>
  Version: {{ version }}
  <readthedocs-version if="external">
    <i class="fas fa-sparkles"></i>
    From a pull request!
  </readthedocs-version>
</div>

No element customization, push to JS API

The above example feels easier as the following pseudo library usage:

readthedocs.on("versionLoaded", (version) => {
  if (version.isOutdated) {
    const elem = document.querySelector('div.version_selector > p');
    // Add icon to elem
    ...
  } else if (version.isExternal) {
    document.querySelector('header').appendChild(...);
  }
});

[humitos]

My understanding here is that the main goal here is to customize the HTML structure of our own elements. Is that correct? I'm saying this because for style we have CSS classes/variables and for logic/data we have a discussion opened at #13

---

I'd personally avoid telling users to customize our elements by using their own JS class as suggested in Element template customization by JavaScript. I think most of our users won't have enough knowledge to do that. Maybe, theme authors, but that is a different conversation, if anything.

The pattern that looks the most intuitive to me is Element template customization by inner HTML. I think it's easy to read by anyone that knows a little of HTML and can be customized with CSS in a simple way too. I'm not sure to understand the "security risk" here since the inner HTML will be written by the author themselves, so, why they would write risky code for themselves? 🤔

Going a little further here, since I like this pattern the most, I'd propose to use <slots> for customizing the HTML structure when possible. It seems to solve the exact problem we are trying to solve: https://lit.dev/docs/components/shadow-dom/#slots

• supports multiple slots on each component
• allows to have default content on them
• slots can be named, so each one is independent from the other
• does not have the problem to run "unsafe" rendering

I quickly tried the following code:

    <readthedocs-notification class="raised toast">
        <div slot="content">
            This content is changed by the user.
            There are variable substitution as well.
            See
            <a href="${this.urls.build}">build's detail page</a>
            or
            <a href="${this.urls.external}">pull request #${this.config.versions.current.slug}</a>
            for more information.
        </div>
    </readthedocs-notification>

and it renders as

So, it works! However, the immediate problem that we can see here is that it's not possible to use Lit expressions (e.g. ${this.urls.external}) on it --which kind of makes sense. Maybe there is something that I'm not aware of and we can make these expressions to work.

In any case, IMO, this approach is pretty clean and we should try to make it work as we want.

[humitos]

OK. It seems I'm not the only one wanting this: https://github.com/lit/lit-element/issues/646. It's currently unsupported for different reason, being the main one XSS. However, there are people saying that this could be implemented by using a "template system" that works inside HTML, like using {{variable}} to replace with it's variable's value. They pointed the original author to what Microsoft does with their own components: https://learn.microsoft.com/en-us/graph/toolkit/customize-components/templates

[humitos]

I think I found what I want after reading a lot on the internet 😅

I arrived at this issue https://github.com/lit/lit/issues/867 and found these two different approaches:

• https://github.com/shaman-apprentice/lit-transformer
• https://github.com/justinfagnani/stampino

I need to make a test now 😄

[agjohnson]

I'd personally avoid telling users to customize our elements by using their own JS class

I would also avoid this pattern for customizing our elements, but would also say that the patterns above are for replacing our elements entirely.

I still see the levels of customization as this:

• Basic: uses a variant element theme class name like <readthedocs-notification class="inverted"> in the project's theme templates. Maybe uses an included CSS file with our CSS variables to match our element's styles to the documentation style. This is still using the elements injected in our shadow DOM though.
• Intermediate: ???
• Advanced: theme maintainer integrating our data from our library directly into their own theme. For example, populating their own version selector dropdown/search.

The majority of users should end up in our basic use case. This level of customization is easier for us to predict and cover with backwards compatibility.

Advanced users will require more direct manipulation of our data, and probably won't need to use our elements at all.

I would put some version of templating as an intermediate customization pattern. It's not clear if we need it just yet though, and I could see us implementing this customization pattern later.

I think it's easy to read by anyone that knows a little of HTML and can be customized with CSS in a simple way too.

I do agree this pattern is easy and quite simple, which I like.

What I don't like is that the template override pattern requires a specific version of our library, as the template context will change as we continue development. A project that customizes the template like this should really be using a pinned version of this library, so that their implementation doesn't deviate from the template context the library is providing.

This is more of a problem with a template in HTML, as a user that doesn't touch their JS probably also doesn't touch their JS dependencies. Users and theme maintainers could still miss this with JS customization though.

It's currently unsupported for different reason, being the main one XSS.

This sounds like the issue I was pointing towards above with unsafeHTML: https://lit.dev/docs/templates/directives/#unsafehtml

Note, the string passed to unsafeHTML must be developer-controlled and not include untrusted content. Examples of untrusted content include query string parameters and values from user inputs. Untrusted content rendered with this directive could lead to cross-site scripting (XSS) vulnerabilities.

I'd propose to use for customizing the HTML

This could be usable, I haven't spent much more time with this, but it does seem like slots would normally be used for appending child elements into our existing shadow DOM elements, not replacing all of our elements with the inner HTML.

I think I found what I want after reading a lot on the internet

Stampino looks like a great solution if we go implement the templating option.

---

Overall, I lean towards providing a JS pattern to allow for advanced usage first, and gauging if another method for customization is required at all.

[humitos]

The more I think about this the less I want people customizing our addons without clear control from ourselves about how to customize them. I would like, following the definitions we used previously in this issue, to re-define these types of customizations as:

• Basic: customize the addons through the WebUI under the project's admin page (see https://github.com/readthedocs/ext-theme/issues/212 and https://github.com/readthedocs/ext-theme/issues/211)
  • Enable/Disable each of the addons
  • Modify their behavior via exposed configurations (docdiff show additions, docdiff show deletions, search hotkey, default search filter, etc). Note that we control what we expose here and it's not hardcoded on the HTML.
  • Select between different pre-defined look&feel (styles) per addon. Notification currently supports: inverted, floating, toast.
  • Don't allow hardcoded HTML attribute like <readthedocs-search trigger-event="keydown">. These will be pretty hard to maintain over time since we don't have control.
• ~Intermediate~: there is no intermediate customization pattern
• Advanced: theme maintainers using readthedocs-addons-data-ready custom event defined in #64
  • Use the data expose by our backend API as they want
  • Create completely new addons to cover pretty specific use-cases

This will reduce the maintenance burden and will give us full control on the addons, allowing us to "break something" and execute a data migration for Basic customizations if needed, while keeping Advanced customizations working since they are pinning the API JSON structure expected.

My proposal is to implement the Basic and Advanced customization first and grow from there based on users' feedback. We are pretty close to get there already 👍🏼

[agjohnson]

Don't allow hardcoded HTML attribute

Perhaps this is our intermediate customization use case? I'm not particularly sold on HTML attribute customization in either direction yet, so I'm willing to not rule this out just yet.

For example, if a theme author wants to customize our addons for their theme users, the HTML attribute option could enable that without getting into JS customization. A theme author might also want to entirely replace our addons, and that is a good case for JS customization.

I definitely agree we should not be pushing normal users towards this solution though. And in the end, there might not even be a strong use case for HTML attribute customization.

customize the addons through the WebUI

Big +1 from me here, I really like the idea of surfacing configuration in our UI.

My proposal is to implement the Basic and Advanced customization first and grow from there based on users' feedback.

Agreed. I think we might want to float the HTML attribute customization around and see what theme maintainers specifically feel about this.

[agjohnson]

So, I am starting to see where an intermediate pattern fits more. We might be hurting adoption by not having an intermediate configuration option, which we might be noticing with not many maintainers working to integrate with out addons/flyout.

Right now, we're only advertising two possibilities to theme maintainers:

• a flyout and addons the theme maintainer have no control over (project maintainers need to configure addons in the UI)
• a flyout and addons the theme maintainers have all the resposibility of maintaining (they have to reimplement everything)

In the middle of those patterns is the theme maintainer that doesn't want to maintain their own implementation but does want to dictate some of the flyout/addons configuration, and for the flyout and addons to visually fit in their theme. I would want to be here as a maintainer.

But right now, we're not giving this maintainer any control. If a theme maintainer wanted to override an addon configuration attribute, they'd have to instruct their users how to edit the RTD addons configuration UI to match their theme's intended usage. This feels like a dead end for adoption for this type of maintainer.

What I'd want as a maintainer is a way to override some of the attributes of the project's addons configuration so that users of my theme immediately had the configuration from my theme, in an easily reproducible way.

I would like to experiment with two patterns:

• "Element template customization by Javascript" above. Maybe just disregard the template customization aspect, this could even just be setting defaults in constructor().
• Using web component attributes to override project UI configuration. This is already supported and native, so this feels quite obvious.

[humitos]

We might be hurting adoption by not having an intermediate configuration option, which we might be noticing with not many maintainers working to integrate with out addons/flyout.

Well, we made a big public announcement and contacted some of the theme authors just a few weeks ago. IMO, the lack of integration doesn't seems to be related with the "complexity" of the integration, but just because we haven't given them enough time yet. Most of the theme maintainers I contacted directly are happy having all the JSON data to integrate it as they want[^1]. Also, it seems they are also taking the opportunity to re-design the integration (in a similar way as we want to do with our theme eventually: split version/language selectors), and that's probably another reason why it's going to take them more time to polish it.

If a theme maintainer wanted to override an addon configuration attribute, they'd have to instruct their users how to edit the RTD addons configuration UI to match their theme's intended usage. This feels like a dead end for adoption for this type of maintainer.

As a theme maintainer I may be able to understand this position, but I would still have all the data to implement it as I want and base my integration on an API call that I know it's not gonna change over time --so, I would stick to that approach. However, the HTML layout or CSS would eventually change and may break my integration.

However, as a Read the Docs maintainer, I'm not sure I want to provide such type of customization to theme maintainers. I want my users to have the full experience when they build their project on Read the Docs for the first time: all addons enabled by default. I don't want to loose the relationship with my users just because the theme maintainer decided to build a different integration on their theme. I want to give the users the ability to disable Read the Docs addons by themselves after seeing how they work and being conscious of that decision [^2]. I don't want to "remove the magic from our side and put the magic on the theme's side". I want to remove this magic and keep these configurations explicit and understandable for our users. IMO, users wanting only the theme flyout, should enable the theme flyout and disable the Read the Docs one. Users wanting the Read the Docs latest notification, should keep it enabled and disable the theme one. I clearly see how a magic mixture of them would end up with pretty confused users.

One of the biggest benefits of the approach we built here is that we can deploy, in a simple way, a new release without re-building docs and without breaking any downstream integration (remember the HTML layout returned by footer_html/ was impossible to touch?). I don't want to follow a pattern where we will be blocked again and we cannot touch the HTML/CSS/JS code because it will break themes that are customizing our own addons. I want full control over our addons.

Let's say that tomorrow we add a slider in the flyout to toggle DocDiff. If the theme is overriding that section we loose the opportunity to deploy this new feature to our users based on a theme maintainer's decision. I don't want to loose that relationship with my users. I want to have the power to communicate to my users the features we are building for them in a clear way.

Following that vision, my position here is that I want to have full control over the addons we expose to our users and I want to give theme maintainers the ability to integrate with Read the Docs as they want on their theme using the API without interfering with our addons. I also want to give documentation authors full control on customization of our addons via the WebUI/dashboard. I want them to have the final decision here.

[^1]: We built the API integration with the JSON-like object pattern based on the feedback we received from them during all these years. [^2]: They could follow the instructions of the theme maintainer for the "suggested experience" as you mentioned before. I'm :+1: with that.

[agjohnson]

I haven't had time to come back to this conversation. I'm not disregarding your notes above, and there is a lot we overlap on there actually, but I did want to note a couple things while the conversation today is fresh:

• I think we're on the same page and both want to try to keep as many projects using native addons actually. We definitely benefit from this and users benefit from this.
• Our current pattern does not support per-version configuration or reproducible builds
• Themes adopting a full customization pattern does already make for a hard split between our latest addons features and the features the theme implements. We only want maintainers doing this if they are very serious about maintaining our features in their theme.
• The more flexible and configurable we make addons the less theme maintainers will need to control addons. There is a trade off on our effort though, and theme maintainers are probably able to maintain these customizations better.
• An intermediate customization pattern would allow themes to use our native addons without pushing the theme to full customization. However this is subject to API drift as builds always inject the latest addons release.
• I think a lot of what you're describing is actually release complexity issues. Allowing themes to pin to a version of addons does address some of that complexity. But, this is indeed still at the expense of not getting features from the latest addons. Similar to theme maintainers building all of our features, this has the effect of lagging feature support. Unlike theme maintainers building our features, theme maintainers can extend our addons in this pattern.

---

To note what I was describing a bit more and touch on that last point while the conversation is fresh:

Release patterns might be a helpful discussion, as what you're describing is the issue we had historically. The trouble has always been the conflict between injecting bleeding edge code and historical builds. Release structure might not be as complex as we're picturing:

• Theme specifies API version
• Theme installs npm install @readthedocs/addons^2.0
• Our injected addons doesn't supersede these already included addons

And yes, this doesn't give the project the latest addons, but this is the same outcome as a theme taking full control and not implementing our features. If we give the theme a way to extend addons, it's more likely it will stay up to date. My preference is leaning towards a pattern that theme maintainers extend our classes I think.

I do agree that giving the user ultimate control of addons makes sense, but perhaps there is a option between theme and user control. Maybe a config option? addons.allow_config_override: true is explicit, reproducible across projects/versions, and it's still a step the user has to take.

These are just some ideas though, not hard suggestions.