Page MenuHomePhabricator

[SPIKE] Proof-of-concept CSS components for HTML.php
Closed, ResolvedPublic5 Estimated Story PointsSpike

Description

To better determine how to proceed in regards to T321354: [SPIKE] Draft scope for CSS-only subset of Codex, let's explore what CSS-only Codex components would look like for a very simple use-case: tweaking MediaWiki's Html class so that the elements produced match the equivalent Codex components. Elements that would need to be supported here:

  • Buttons
  • Checkboxes
  • Text Inputs
  • Messages
  • Radio buttons
  • Dropdown/Select (for Namespace Selector)

This work should be done as a proof-of-concept patch, and will probably never get merged (at least in its initial form). Instead, this code should help to inform the discussion around T321354: [SPIKE] Draft scope for CSS-only subset of Codex and its parent – does this approach make sense, are there any drawbacks to consider, etc.

Some specific questions that should be considered here include:

  • How does this integrate with the design system?
  • How could this impact with the architecture of the design system?
  • How does this integrate with mediawiki?
  • How far in scope does this need to be?
  • What is the cost to build? What is the cost of maintenance?

The VueTest special page can be used as a sandbox to compare the HTML/CSS components with their JS equivalents.

Event Timeline

egardner created this task.
egardner changed the task status from Open to In Progress.Nov 28 2022, 9:33 PM

Change 865777 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] [PoC, do not merge] Message: Add CSS-only version

https://gerrit.wikimedia.org/r/865777

Change 865778 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] [PoC, do not merge] Radio: Add CSS-only radio

https://gerrit.wikimedia.org/r/865778

I've pushed a few proof of concept patches and have some thoughts about implementation on the template/CSS side of things. You can check out patches here:

Note that I also added demos to the Sandbox so you can look at the CSS-only versions there as well.


Notes for the proof of concept patches

  • Since we don't have a CSS-only icon component yet, I just stored SVGs in variables and used them as background images (this is what we're currently doing for the check icon in the Checkbox component)
  • In the patches, I just created a component-name.less file in the component's folder, but that's probably not where we'll want this to live long-term.
  • Similarly, I just tacked on the .less file to the component's demo page or Sandbox component. This means the styles won't be properly applied elsewhere (e.g. except on the Radio demo page, radio styles will be broken in the controls). My goal was to focus on the markup and styles, not the build.

Overall impressions

I'm really happy with how easy it is to use the CSS-only versions of the components. The markup is simple and it's cool to see design system styles applied to this new JavaScript-free implementation. From the developer user's perspective, I think these CSS-only components are straightforward to work with and similar enough to mediawiki.ui.

In terms of open questions related to implementation of the base styles, I think we just need to consider how to best implement the components in Codex so that the files are still readable and maintainable.

Markup simplicity and DX of CSS-only components

I wanted the markup and CSS class names of CSS-only components to be as simple as possible. To that end, I made these decisions:

  • Using the base form of the component class name (e.g. .css-select) for the CSS baseline, i.e. the CSS-only component
  • Adding -vue to the Vue version's class name for anything specific to that implementation (more on this later)
  • Refactoring the Radio component to make the <label> and <input> elements siblings, rather than having the <label> element wrap everything. I initially had the label wrapping everything so we could avoid the need for a for attribute on it and an id on the input, but the mediawiki.ui radio has them as siblings with those attributes, and having them as siblings removes the need for an extra span wrapping the label text. In the end, I think the new markup is simpler. We may need to do something similar for other components when creating the CSS-only version.

Class naming structure

As mentioned above, I wanted the CSS baseline to use the implementation agnostic .cdx-component-name. I decided to do the following for the Vue versions:

  • Add a class name to the root element with the -vue suffix, e.g. .cdx-message-vue. I originally used a modifier, .cdx-message--vue, but thought that was unnecessary and a bit misleading. The outermost selector in the styles in the .vue file will be that implementation-specific selector.
  • Update other class names in the component only if they are specific to the Vue implementation. This makes the CSS baseline nice and clean, since styles can apply both to the CSS-only version and the Vue version (e.g. the background and border colors for block-style messages). However, it means there is a mix of classes within the Vue implementations (check out Message.vue for an example). I'm unsure if this actually makes it clearer what comes from the CSS baseline and what is implementation specific, or if it's more confusing.
  • This really broke down in the Select component, more on that next...

The Select component

We chose this for a proof of concept since we knew it would be especially tricky, and indeed, it turned out to be quite problematic.

The main reason for this is that for the CSS-only component, you need to style the <select> element, which is the outermost element of the component. However, for the Vue component, these same styles are applied to the .cdx-select__handle element, not the root of the component. Therefore, we can't use the cdx-select class for both the <select> and the root element of the Vue component.

I see two potential solutions:

  • In the patch, I moved the cdx-select class to the handle element. This makes the Vue template and styles pretty confusing (e.g. you might wonder where the transition styles come from if you just look at the Vue component), and means we can't easily apply nested styles in the CSS baseline.
  • Since we're only styling the <select> element for the CSS-only version, there actually aren't that many styles needed. We could just accept that the styles don't overlap enough and keep them separate. We could create a mixin for the styles that actually are shared and apply it to both .cdx-select and .cdx-select-vue__handle. The cdx-select class would not be applied in the Vue version.

A secondary but related reason that this was difficult is that for the CSS-only component, we will rely on the <select> element's :enabled and :disabled states, while we can't do that for the Vue version and must apply dynamic classes. So we can't cover enabled/disabled styles for the Vue version in the baseline CSS without bringing in Vue-specific selectors, which I think we should avoid.

Potential solutions:

  • In the patch, I just duplicated these styles
  • We could create mixins for these styles

Other components and overall effort

As we create CSS-only versions of other components, we are likely to run into similar issues and need to do some refactoring to make it all work. That said, doing these 3 components took me a day and a half, so I don't think the amount of effort needed would be that much, especially if we're limiting CSS-only versions to a select group of components.

Thanks @AnneT, this is really informative.

Regarding the naming conventions we want to establish here – is it really necessary for developers to add a -vue suffix to the classes that only show up when a JS component is active? Why not just limit the <style> blocks of single-file components to only include these classes (they could be left out of the stand-alone CSS files); then they will not be present unless a JS version of a component is initialized on the page. This would save developers from having to keep track of which styles are Vue-only vs which styles also get used in CSS (and might also save us from sending some redundant styles to the user)

For the message example, that might look something like this:

const rootClasses = computed( (): Record<string, boolean> => {
    return {
        'cdx-message--inline': props.inline,                    // this style would be defined in CSS
        'cdx-message--block': !props.inline,                    // this style would be defined in CSS
        [ `cdx-message--${props.type}` ]: true,                 // these styles would be defined in CSS
        'cdx-message--user-dismissable': userDismissable.value  // this style defined here in the Vue SFC
    };
} );

Then in the style block of the same file, you would only add definitions for .cdx-message--user-dismissable since those styles are only relevant when the component is fully interactive.

.cdx-message {
    // most styles are defined in CdxMessage.css
    // only Vue-specific stuff is defined here
    &--user-dismissable {
        padding-right: @min-size-base + @spacing-100;
    }
}

I guess I don't see the advantage in separating out the Vue-specific classes via a suffix, since we are already able to ensure that they only get loaded along with the JS components. Am I missing something?

@egardner Good point—your proposal would work for features that are only present in the Vue version, but not when different styles are needed for the same element in both the CSS-only and Vue implementations. One example is the message icon: in the CSS-only version, the element with the .cdx-message__icon class needs a background image and other background styles, while in the Vue version it contains an svg and just needs some color styles. If we apply background styles to .cdx-message__icon, then we'd have to unset them in the Vue implementation.

That said, instead of including a top-level class with -vue, we could include a vue-specific class for those particular elements. So the icon element inside Messave.vue could be .cdx-message__vue-icon or .cdx-message__icon--vue or something. That would mean that most components could be simplified in the way you described, and we could just handle the outliers by adding a Vue-specific class name.

I think I'll push an alternative version of my patches implementing something like this, so we can see what it looks like in practice.

Change 866509 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] [PoC, do not merge] CSS-only components, but all inside the SFC

https://gerrit.wikimedia.org/r/866509

Per @egardner's suggestions, I've created this patch, which aims to remove vue-specific class names as much as possible. It also moves all the styles back into the SFC, and accepts that the Select component implementations are too different and should just rely on mixins.

We've also discussed using some kind of --js-active class instead of --vue, but I think we have some outstanding questions about SSR to discuss before we can determine the best route to take there.

One more thing I wanted to mention about the components generated in HTML.php, which use mediawiki.ui class names, is that there are a lot of styles for medawiki.ui components that could be considered mediawiki- or skin-specific (example in mw.ui.button). These styles were added over the years as bug fixes or improvements. If we wanted to replace mediawiki.ui components, we would need to figure out what to do with those styles in order to avoid regressions.

This might be as simple as a .less file in core that targets .cdx- classes and applies these styles as needed. Just something to keep in mind.

egardner set the point value for this task to 5.Dec 9 2022, 6:02 PM

PoC of a few components completed and discussed.

Change 866509 abandoned by Anne Tomasevich:

[design/codex@main] [PoC, do not merge] CSS-only components, but all inside the SFC

Reason:

Abandoning in favor of new patches

https://gerrit.wikimedia.org/r/866509

Change 865777 abandoned by Anne Tomasevich:

[design/codex@main] [PoC, do not merge] Message: Add CSS-only version

Reason:

Abandoning in favor of new patches

https://gerrit.wikimedia.org/r/865777

Change 865778 abandoned by Anne Tomasevich:

[design/codex@main] [PoC, do not merge] Radio: Add CSS-only radio

Reason:

Abandoning in favor of new patches

https://gerrit.wikimedia.org/r/865778