Page MenuHomePhabricator

[Audit] Review of new Vue+Codex New Pages Feed
Closed, ResolvedPublic5 Estimated Story Points

Description

Following up from T340063, Design-System-Team would like to collaborate with Moderator-Tools-Team on an audit (rather than explicit code review) of the Vue+Codex built New Pages Feed experience.

The target outcome is not necessarily specific tasks that need to be completed in time for releasing the new experience (unless a high priority bug is uncovered), but rather recommendations and guidance on Vue and Codex best practices for this and future experiences.

Event Timeline

CCiufo-WMF set the point value for this task to 5.Aug 8 2023, 5:02 PM
CCiufo-WMF moved this task from Needs Refinement to Up Next on the Design-System-Team board.

@AnneT

Note that ORES is an optional dependency that is configured in production and adds quite a few ui elements. Below is a snippet from my LocalSettings.php that may help you have a more complete local environment:

// PageTriage
$wgPageTriageEnableEnglishWikipediaFeatures = true;
$wgPageTriageDraftNamespaceId = 118;
$wgExtraNamespaces[ $wgPageTriageDraftNamespaceId ] = 'Draft';
$wgExtraNamespaces[ $wgPageTriageDraftNamespaceId + 1 ] = 'Draft_talk';
$wgExtraNamespaces[ 999 ] = 'Wikipedia';
$wgMaxArticleSize = 100;
$wgPageTriageEnableOresFilters = true;
$wgOresWikiId = 'testwiki';
$wgOresModels = [
	'articlequality' => [ 'enabled' => true, 'namespaces' => [ 0 ], 'cleanParent' => true ],
	'draftquality' => [ 'enabled' => true, 'namespaces' => [ 0 ], 'types' => [ 1 ] ]
];

Below are my audit findings. Overall, the code looks excellent, and it's exciting to see that Vue and Codex could be used to make the code more maintainable and to unify at least some of the styles, UX, and a11y features with the design system.

All of these recommendations are optional, and changes should only be made if they are acceptable to the users and developers of this page. Many of these recommendations could be done later, or when the appropriate thing is available in Codex. I have placed a before items I believe are fairly important to change now.

Potential Codex usage

Components
  • CdxCheckbox
    • Using CdxCheckbox would mean the LabeledCheckbox component could be removed altogether, and pairing the input id with the label's for attribute would be handled automatically
    • Wouldn't need to maintain a hardcoded list of acceptable messages in LabeledCheckbox
    • Would get interactive styles that match the radios
  • CdxToggleButton
    • The "set filters" toggle should probably be a CdxToggleButton
    • If unsetting the CdxToggleButton styles is too much, you could instead follow the toggle button pattern defined in the WAI APG by adding role="button" and :aria-pressed="settings.controlMenuOpen" to the existing toggle
    • Would also recommend changing that element from <b> to a <span> with a class to target for font styles
  • Things I don't see as worth using at this point:
    • CdxSelect: the current <select> element is sufficient if you just want UX/a11y features and not Codex styles
    • CdxTextInput: current usage would only get you Codex styles, which you would have to undo. If you ever want to use additional features of CdxTextInput (icons, clearable, Field features), you could make the switch then.
  • Future components:
    • CdxPopup for the filters popup
    • CdxFieldLayout or something like it for multi-input field layout
Design Tokens
  • General: You could create a .less file with all of your custom variables, then use those throughout the code. This would make it easier to identify and swap out for Codex tokens in the future
  • Border:
    • You could use @border-width-base and @border-style-base
    • Recommend not using color tokens for border-color if possible (e.g. here)
  • Color
    • Recommend replacing hardcoded hex codes with tokens when there's an exact or very close match (e.g. #000)
    • Recommend trying to use the appropriate token category, e.g. @color- tokens for color rules, @background-color- for background-color rules, etc.
  • Font
    • Recommend trying to use @font-size- and @line-height- tokens if possible
  • Spacing:
    • @spacing- tokens are available and recommended. They are in ems, which we recommend for spacing in order not to scale the entire UI when font size increases. However, swapping out spacing tokens for existing em values (e.g. using @spacing-100 instead of 1em) will slightly increase the resulting values
    • You could change multiple values to approximate the existing sums of values. E.g. you could bump up the margins here while reducing the padding here
    • Perhaps consider this if you feel like an overhaul of the style architecture is warranted. Otherwise, you could wait for if/when base font size in Vector increases to 16px
  • Z-index:
    • Recommend using the Codex z-index scale if possible
Composables

You could use the useModelWrapper() composable to handle stacked usage of v-model.

Potential design improvements

  • Add &:hover { cursor: @cursor-base--hover; ) to interactive elements, especially in the filters popup. CdxRadios already have this (pointer cursor when hovering over the input or the label), so unifying that behavior across the inputs would be nice.
  • NppFilterRadio radio + text input combo
    • If you set the screen reader text mixin on .cdx-radio__label instead (this line), and add back the margin-left on the input, the spacing will be better
    • This should probably be wrapped in a <fieldset> with a visually-hidden <legend> with the label "Username", plus a visually hidden label for the input "Username value"
    • Can just wait for a Codex field layout to handle this
  • Can you use flexbox for article row layout styles? If so, you can more easily get the alignment of the icon, article info, and info chip aligned
  • Accessibility recommendations that you can take or leave:
    • "That" and "Were created by" labels are not ideal for screen reader usage
    • It would be nice if clicking outside of the popup closed it (maybe wait for a CdxPopup component)

Potential code improvements

  • ListFilterMenu has a couple uses of the deprecated type prop of CdxButton, recommend changing to weight (although I'd also recommend not having two primary buttons)
  • We don't really recommend using the class cdx-link, but rather applying the mixin to a feature-specific class

Codex improvements

  • Compact theme/user styles
  • Alignment of radio/checkbox label at 14px font size
  • Add CdxPopup component
  • Add field layouts

I've met with the Moderator Tools engineers to discuss these findings and receive their feedback - closing this out!