Change #1131336 had a related patch set uploaded (by Samtar; author: Samtar):

[mediawiki/extensions/TemplateData@master] [WIP] ext.templateData.templateDiscovery: Add favorite button to template pages

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

Change #1131336 merged by jenkins-bot:

[mediawiki/extensions/TemplateData@master] ext.templateData.templateDiscovery: Add favorite button to template pages

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

I have to point out that the "Mark as favorite" button is visually imbalanced with the "Watch" button. The latter uses a 16x16px icon, whereas the former is 20x20px.

Current appearance:

Proposed (bookmark is 16px):

I know that the size difference is due to the fact that the "Favorite" is rendered using OOUI styles, but this size difference really bothers me while I look at the top bar.

Good point. I've added it to T396678 because it's a similar issue with Timeless and they can perhaps be handled together.

This feature may've been enabled in production a bit too soon. It feels a bit unready from initial appearance and first use. A couple things I noticed:

1. Layout bug, double border, and button is seemingly unbounded in size.
2. Design issue: Icon is out of style with the rest of Vector.
3. Design: Auto-hiding post-action notification bubble appears to use a style that is rather prominent, and unlike other auto-hiding notifs in the platform. A few examples of such platform features: Watch star toggle, Preferences save, Marked as patrolled, Edit save, TempUser creation, Special:Watchlist inline unwatch confirmation, CentralAuth auto-login, Vector 22 appearance menu close, and ULS language change.
4. Usability: There is seemingly no hover state.
5. Usability: There is seemingly no indication that something is happening.
6. Usability: The action can't be performed in a new tab (link lacks href).
7. Performance: The button is inserted with client-side JavaScript. This contrary to MediaWiki Engineering guidelines as we try to avoid layout shifts during the page load. mw.util.addPortletLink is for user scripts, gadgets, and short-term experiments, not first-party features deployed and enabled by default. Elements visible above the fold should have its layout rendered (or reserved) server-side.
   • This means there can be a noticable 2-3 second delay, where the icon appears after everything else has loaded, distracting the eye by shifting the layout left. This repeats itself on every page view.
   • This means the action cannot be initiated until after this delay. Unlike other actions which can be initiated at any time when the page is visible. If you arrive at a page with intent, this delay can be quite noticable.
8. Performance: There is no busy/disabled state, thus a double click performs the action twice.
9. Accessibility: Related to point 6 and 7, there is no href fallback. This means the feature is only discoverable and available if, when, and after Grade A is available and succesfully applied on top of a given pageview. Thus on modern browsers when using a slower connection or when there is interference from a browser plugin, or when using an older browser, the feature effectively withheld from the user (examples, ref 1, ref 2). Note that I'm not referring to someone purposefully turning off JavaScript.
   • Features are usually shipped as a linkable special page first (very little code, mostly declarative; builds on platform automation and provides perf, security, accessibility essentially for free; with very little testing needed given no frontend or other custom code). Then, depending on resourcing and priority, a JS-based progresive enhancement is sometimes added on top.

Example: Flash of unstyled content (Video):

Example: Modal confirmations.

It looks like this design was borrowed from Codex "success" messagebox, as used in cdx-message--success and mw-message-box-success. This is used quite sparingly at the moment (If this is a new design direction starting with this feature, let me know and I'll discuss it elsewhere.)

If I recall correctly, when we adopted this design around ten years ago, it was to avoid placing undue weight on a non-actionable and optional confirmation message. In particular because the now-completed action was modal. The rest of the interface remains available to the user, and so no action from the message is required to proceed. The message is optional to read, and automatically hidden, further emphasizing that it isn't expected to be read every time. It quickly acknowledges the action visually, to feel responsive, especially when a feature is used for first time.

This is in contrast to non-modal worflows, where the entire page is a confirmation page, or where the message is non-dismissable and expected to be mandatory reading for the user to understand what happened before they move on. There's a handful of examples where we do use this, but this feels different from those:

• Preferences save confirmation when saving without JavaScript. The submission may take several seconds to load, during which the viewport may be blank. After rendering the form again, it looks identical to the previous page, except for this one message so the extra attention may be warranted to stand out after that gap (not animated in real-time).
• When the MediaWiki web installer is completed. Full page confirmation message, with a button inside to proceed to the next step, expected to be clicked.
• When changing your account password, full page confirmation message.

A few of the design layout, and usability issues will be fixed by a patch that's currently in review for T396678: Favorite icon size and alignment is wrong in some skins.

The 'success' type for the notification can easily be removed. I've opened T397808 for that.

The biggest issue is I think server-side rendering. We were going to do that, but it meant we'd also have to add a special page form for changing the favorite status, and that was thought to be too much work for a front-end-only feature like this. However, it sounds like we were wrong, and that approach is the correct one. I'll open tasks for getting it done.

It looks like we should undeploy for now, at least until the above layout fixes are in place and have been deployed.

This means the feature is only discoverable and available if, when, and after Grade A is available and succesfully applied on top of a given pageview.

Although this is true, there's also currently no way to *use* the feature (after you've favorited a template) outside of a Grade A environment. The only thing it does is affect the list of templates shown inside a few JS-only dialogs when editing.

A Special page that listed the favorites (and allowed editing the list) seems worthwhile as a matter of principle -- and to support e.g. bulk-adding cases for initial setup -- but I'm not sure it's going to be particularly helpful in the primary editing use-case.

In T390019#10947722, @DLynch wrote:

This means the feature is only discoverable and available if, when, and after Grade A is available and succesfully applied on top of a given pageview.

Although this is true, there's also currently no way to *use* the feature (after you've favorited a template) outside of a Grade A environment. […]

You're absolutely right. I didn't quite understand the broader context of the feature. And indeed, in terms of "who", there is indeed no use case to provide this capability to a Grade C "user".

I would argue however that it is less about the "who" and more about "where" and "when". Every pageview starts in Basic with JavaScript optionally arriving at a later time, the futher we go the narrower the funnel (intro, guidelines).

From § Frontend performance practices:

Use fixed, predictable, or visually contained layouts by presenting information and interfaces such that their dimensions are constant in HTML or CSS. This avoids moving or pushing down elements after they became visible […].

We enforce that modules cannot affect the initial rendering of the page, particularly "above the fold" […]. This is handled by ResourceLoader […], which processes the module queue asynchronously.

As such, the distinction quickly turns away from a "who"., to a moment in space and time. When you're on a slow connection and navigating around to find a specific article or factoid, you may experience most of your pageviews as Grade C, as JS won't initialize in time.

Suppose we generalize a bit, ignore the edge cases, and focus on the Grade A "user" only. There is still an issue here. They open a page, the browser rendering settles, and then this feature comes in and shifts the layout to add an extra button. The context that this template favorite button is modifying, is a server-rendered context that loads upon page navigation. This isn't inside VisualEditor or elsewhere within a model context where JavaScript has already arrived. As such, it creates a subpar performance for the end-user. We could tell them that they didn't "need" it earlier before the Grade A payload, but that's missing the point.

One solution to this would be to render only the link server-side, keep the rest in JS, and use .client-nojs in CSS to hide it on pageviews where we know the JS payload won't arrive.

I hope this explanation makes more sense :)

In T390019#11071234, @Krinkle wrote:

In T390019#10947722, @DLynch wrote:

Suppose we generalize a bit, ignore the edge cases, and focus on the Grade A "user" only. There is still an issue here. They open a page, the browser rendering settles, and then this feature comes in and shifts the layout to add an extra button. The context that this template favorite button is modifying, is a server-rendered context that loads upon page navigation. This isn't inside VisualEditor or elsewhere within a model context where JavaScript has already arrived. As such, it creates a subpar performance for the end-user. We could tell them that they didn't "need" it earlier before the Grade A payload, but that's missing the point.

One solution to this would be to render only the link server-side, keep the rest in JS, and use .client-nojs in CSS to hide it on pageviews where we know the JS payload won't arrive.

Seconding this. This is exactly when we should be using .client-nojs, JS dependent functionality that without it would cause a layout change.

It sounds like the appropriate thing to do here might be to load a disabled button initially (with all DOM and styles), and then enable it when JS loads. That way there won't be any layout change, but nor do we need to build a non-JS process for a JS-only feature (e.g. adding Special:TemlateFavoriting as an HTML form for changing the favorite state).

There's also recently been talk of removing this new button entirely, although I think then we might want it to still exist in the tools men (where the same flow could apply: first load it with no interaction possible, and then add functionality when JS loads).

Anyway, this task is closed, so I've created T401544 to talk about how to fix things up.