Page MenuHomePhabricator

Typeahead search deployment: Use CdxTypeaheadSearch in Vector
Closed, ResolvedPublic

Description

Summary

A Vue 3 version of TypeaheadSearch has been added to Codex (see demo). This task covers removing the WVUI version of TypeaheadSearch (WvuiTypeaheadSearch) from Vector in favor of the Codex version (CdxTypeaheadSearch).

Implementation details

Since Codex is built with Vue 3, implementing the CdxTypeaheadSearch component will require updating the app in Vector that uses TypeaheadSearch from Vue 2 to Vue 3.

In addition to using the CdxTypeaheadSearch component, we will need to update styles in Vector that are applied to a server-rendered version of the search box that make it mimic the appearance of TypeaheadSearch. Since some design changes were made while developing CdxTypeaheadSearch, some of these styles will need to change accordingly to match Codex values and tokens.

Finally, we will remove references to WVUI throughout the codebase, replacing them with generic "Vue" terms or Codex references.

Acceptance criteria

  • Vector search app is updated from Vue 2 to Vue 3
  • Dependence on wvui ResourceLoader module and npm package(s) replaced by codex
  • Use @wikimedia/codex-search build
  • WvuiTypeaheadSearch is replaced by CdxTypeaheadSearch in the search app
  • WVUI URL generator code is moved into Vector, since it is app-specific
  • Server-rendered search box styles are adapted to work with Codex styles
  • WVUI references in Less code are updated to point to Codex
  • VectorWvuiSearchOptions config variable has been updated to the more generic VectorVueSearchOptions (this is done in the Vector code and does not need a corresponding patch in mediawiki-config since VectorWvuiSearchOptions was never included there)
  • Functional testing of CdxTypeaheadSearch and its subcomponents has been completed (Codex-specific)
  • QA of the implementation of CdxTypeaheadSearch in Vector has been completed (Vector-specific)

https://phabricator.wikimedia.org/T303558#8110679

QA Results - Beta

ACStatusDetails
122 x ✅ , 3 x ⬜ , 1 x ❌see T303558#8110679 for breakdown. a bug was filed for the error that was found.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
AnneT changed the task status from Stalled to In Progress.May 31 2022, 4:44 PM

@AnneT can you clarify the status of this ticket

@DAbad I've updated the task description to include more information, acceptance criteria, and information about the acceptance criteria that have not yet been completed.

Change 804013 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/extensions/CirrusSearch@master] article_page test: Change WVUI CSS selector to equivalent Codex selector

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

@Sarai-WMDE @Volker_E: from a usability perspective I think the main concern, or most important goal, is the ability for people to easily scan the list of article titles, in order to find the one they are looking for. A secondary goal is verifying that the article title they initially locate is the one they want, which is where the article descriptions (and images) become useful.

We previously agreed upon reducing the font-size of the article titles from 16px to 14px (which is the same font-size used by the article descriptions). I think this was a good change for general consistency, and also because I think 16px is too large for interface elements. At the same time I also worry that this change makes it more difficult for people to easily scan the article titles, because there is less contrast between the article titles and the article descriptions. Compare:

previouscurrent
image.png (1×1 px, 325 KB)
image.png (1×1 px, 306 KB)

Therefore I am wondering: what changes can we make in order to make it easier for people to scan the article titles? Generally speaking I think we can achieve this goal by having greater contrast between the article title and article description, and possibly by adjusting spacing. Three explorations:

  1. change the article descriptions from #54595d to #72777d (which, as far as I can tell, still passes AA WCGA accessibility requirements):
article descriptions #54595darticle descriptions #72777d
image.png (1×1 px, 306 KB)
image.png (1×1 px, 303 KB)
  1. reduce the font-size of the article descriptions from 14px to 13px:
article descriptions 14pxarticle descriptions 13px
image.png (1×1 px, 306 KB)
image.png (1×1 px, 299 KB)
  1. adjust spacing so that article descriptions are grouped more closely with article titles
current spacingarticle descriptions shifted up by 2px
image.png (1×1 px, 306 KB)
image.png (1×1 px, 304 KB)

I propose that we make one, more multiple, of these changes in order to increase the ability for people to scan the list. What do you think? Also, to note, I don't see this as a blocker to deployment (though at the same time if we can all agree it seems like a relatively easy change to make).

small thing: if you look closely at the bottom of the input box when it is focused you will notice that in production it's 2px blue, and in codex it's 1px blue and 1px gray. I think it looks better with 2px blue:

image.png (1×2 px, 296 KB)

Thanks @alexhollender_WMF for the quick and close look!
I think Option 1. with lowering slightly is the best weighing the different needs of readability, ability to quickly skim.

small thing: if you look closely at the bottom of the input box when it is focused you will notice that in production it's 2px blue, and in codex it's 1px blue and 1px gray. I think it looks better with 2px blue:

For the second quest, we've shortly had a discussion about this in the Design System Designers circle, and given that elements should represent a specific elevation (signified by a box-shadow and interaction with other elevations in our faux 3d surface, the menu comes on top of the focus of the triggering element.
That's the consistent approach by all similar components and it seems acceptable to me to follow that interface logic on TahS as well.

Note that in contrast to TahS, in Codex we might have dropdown menus that are also opening to the top, so we would have to go through extra hoops for the removing the top, or in those cases bottom border.

Thanks (again) for sharing this really sensible concern, and for already providing solutions, @alexhollender_WMF! 🙏🏻

My comments about the options shared:

  1. Change the article descriptions' color from #54595d to #72777d: As Volker mentioned, this could be a simple and effective solution. Nevertheless, we documented #54595d as our "subtle" content color (see palette) and applied it to descriptions in menu items because it still gives enough contrast to descriptions' text when menu options are hovered (they display a #EAECF0 background). Unfortunately, #72777d (which is documented for use with placeholders and disabled text) doesn't have enough contrast against that gray background. We could decide to iterate on the hovered background color of menu items, but this would require separate discussions and some exploration.
  1. Reduce the font size of descriptions: We should use small text really sparingly. 13px is really tiny for the web if we want to ensure readability. So, even if the result would be effective, I would initially not suggest this.
  1. Reduce spacing between title and description: I agree that spacing contributes to making the scanning of the menu a bit "syncopated". We unfortunately introduced that extra spacing when we replaced custom menu items by their standard version in TahS. Said menu items currently have a line-height of 1.6 applied to them, which creates that extra separation between the lines of text. Nevertheless, if everything goes as expected, we'll be reducing that line-height when typography tokens are added to the Codex design system (probably to 1.4, but this is still to be fully decided). I'd hope this reduction will contribute to group together titles with their descriptions, and improve the ability of users to scan through menu options. This is how the menu would look like at 1.4 line-height:
Current Codex TahS component with consistent MenuItemsVersion with future line-height
Screenshot 2022-06-09 at 17.55.38.png (1×1 px, 409 KB)
Screenshot 2022-06-09 at 17.48.42.png (1×1 px, 398 KB)

Do you think that this last change would suffice to make the component fulfill the described goals?

Now, regarding your last comment:

small thing: if you look closely at the bottom of the input box when it is focused you will notice that in production it's 2px blue, and in codex it's 1px blue and 1px gray. I think it looks better with 2px blue:

I agree. We discussed this pattern internally a while ago with other designers and agreed to make menus look adjacent to inputs instead of overlapped with them (Task: T307767: [Menu] Make menus look adjacent to inputs (Needs discussion)). We also ran the proposal through engineers and even documented a potential solution. Of course, after reading @Volker_E's comment, I realize we might need some further internal discussion regarding this, so we'll do that and get back to you asap.

For what it's worth, reducing the line height of menu item content was part of the solution to T306525, which aims to improve the alignment of icon/thumbnail with menu item title. You can check out a demo of TypeaheadSearch with this new line height. Note that this is still open for review/additional feedback.

Indeed! I forgot to ping you about that, @alexhollender_WMF. The content within options has been grouped a bit more closely (option 3) as a result of a recent patch, as Anne just commented. Please let us know if you think this new version improves scannability to some degree.

@Volker_E @Sarai-WMDE @AnneT thanks for your responses.

Regarding the first point:
@Sarai-WMDE I do think that the updated line-height makes the menu easier to scan. The goal I stated is difficult to evaluate without testing, so it's difficult for me to answer your question "Do you think that this last change would suffice to make the component fulfill the described goals?". My intuition is that we should keep pushing on this, and make additional changes so that the list is even easier to read/scan. I am curious, do you think it satisfies the general usability goal? From a process perspective I expect your team to have higher standards than my own when it comes to accessibility of interface elements, so ultimately I am happy to defer to you : ) I wish I had more time to engage with the details of your comments but for now one thought:

  • as we discussed briefly in Slack I do think it makes sense for links within menus to be blue, so that they are consistent with other links and most discoverable. I have not had time to propose this in Phab yet (I hope to do so in the next few days). interestingly this situation might provide additional evidence for why this might be a good pattern, because the titles then stand out more from the descriptions:
    Screen Shot 2022-06-22 at 4.14.07 PM.png (679×701 px, 101 KB)

Regarding the second point:
@Volker_E I understand the logic of the menu being on a higher z-index than the input, however I question whether that has any benefit to the user experience. If it does not have any benefit to the user experience, and we agree that it looks worse, then I don't think it's the right decision.

Regarding the updated line-height:
This is difficult to measure but as far as I can tell the Title + Descriptions are 1px below the vertical center of the thumbnail image. If it's not possible to center them exactly I think it would be preferable for them to be 1px higher than 1px lower.

Screen Shot 2022-06-22 at 3.59.43 PM.png (851×971 px, 68 KB)

Thanks for your reply, @alexhollender_WMF! I think my question should have been phrased differently. Knowing that your team has tested and iterated on the component before, I think the existing TypeaheadSearch has been considered a source of truth. A sort of standard we're aiming for. The change in line-height was a way to try to maintain the same level of scannability in the new component. Therefore my question should have had a more comparative tone, related to the satisfaction of said standards, which already fulfilled by the production component.
I agree with you. In my opinion, the new version makes it easier to parse through the options in the menu. We weren't planning on conducting any usability testing as part of the implementation of this component, for the reasons stated above, but if we were to introduce (more dramatic) changes to the original designs, it'd make sense to then introduce this step in our process.

In any way, thanks for your trust! The DST might aim at having high standards, but nothing can beat collaboration, and any improvement suggestions (like you just did in that last comment!) will have a great impact in our projects.

Regarding:

  • Changing the color of options' titles to blue in navigational menus: no strong objections, as I like the semantic side of the proposal. A small concern worth mentioning would be, though, that this could give the impression to users that only the title is clickable, instead of the whole option? 🤔 Something worth discussing (and probably testing!) for sure. I hope we can dedicate a session / Phab thread to investigate this.
  • Misalignment between thumbnail and option content: You're absolutely right! This is indeed related to the change in line-height, but also to the change in the flex alignment (which is now top to improve readability of menu items with multiple lines of text T306525). The good news is that the problem goes away when the font-size is reduced to 14px (so the problem won't be visible in Vector):
Menu item at 14pxTahS at 14px
Screenshot 2022-06-23 at 12.47.11.png (244×1 px, 101 KB)
Screenshot 2022-06-23 at 12.48.19.png (1×1 px, 355 KB)

The bad news is that all the solutions I tried to apply to solve the issue in Codex (i.e. at 16px) for now (reducing the line-height of the description, add a small margin to the thumbnail..) have a negative impact in the 14px version. I'll keep trying and keep you posted. Thanks so much again for detecting this issue 🙏🏻

  • Changing the color of options' titles to blue in navigational menus: no strong objections, as I like the semantic side of the proposal. A small concern worth mentioning would be, though, that this could give the impression to users that only the title is clickable, instead of the whole option? 🤔 Something worth discussing (and probably testing!) for sure. I hope we can dedicate a session / Phab thread to investigate this.

yes, interesting point I have also been considering. I wonder what the consequence of this misconception would be, and if, once they start moving their mouse towards the page title and see the whole item/row get highlighted with the hover state + cursor change, it will become clear that it is clickable? I've opened a task here: T311270: Evaluate navigational menu link/item styling.

I think that this might be the strongest point against the new navigational menu item styling proposal: the whole menu item is the interactive element here, the one that needs to react to hover and click events. In the markup, this is currently expressed by the fact that all the content in menu options (thumbnail, suggestion text) is wrapped in an <a> element (not only the title, which is currently not a link). I might be wrong, but moving ahead with the proposal would either involve applying link styling to an element that's technically not a link, or needing to nest two consecutive links with the same href.

but moving ahead with the proposal would either involve applying link styling to an element that's technically not a link, or needing to nest two consecutive links with the same href.

I think a different way of thinking of it is: if the entire element is a link, then the title and description text should be blue by default. Instead, we are currently suppressing the link styling in order for the page title to be black and the description gray. With this change we would instead only be suppressing the link styling on the description text (still displaying it in gray). I think from that perspective we are then using less CSS, and overriding the default styling less.

I think that this might be the strongest point against the new navigational menu item styling proposal: the whole menu item is the interactive element here, the one that needs to react to hover and click events.

I agree with @Sarai-WMDE and as I've commented in T311306 and T311270 I think we should use link behavior just for text links within block texts (e.g. links within the article). For the rest of clickable items (menu items, menu triggers, tabs...) we should use other components (not links) and create for them a different behavior in which the whole item is clickable (painting all the bg as we have in our current menu items).

I think a different way of thinking of it is: if the entire element is a link, then the title and description text should be blue by default. Instead, we are currently suppressing the link styling in order for the page title to be black and the description gray. With this change we would instead only be suppressing the link styling on the description text (still displaying it in gray). I think from that perspective we are then using less CSS, and overriding the default styling less.

I totally see your point, Alex. That's very true and logical. I guess one could say that suppressing the style for all text has the purpose of concentrating interaction at a different level: the entire element. This is in line with what Bárbara is describing, and I'm finding a similar case while working on the Card component: interactivity and navigation are represented and facilitated by the whole card, and the elements inside don't need to be singled out to reinforce an idea of navigability given that this is a very common and expected pattern (like search!).

@bmartinezcalvo @Sarai-WMDE I am not yet able to see how styling links in black makes the interface easier for people to use; can you help me understand that? what is the issue with them being blue?

I think there is a good case for blue, as we don't need to introduce another style. By keeping them blue we have greater consistency, easier understanding of usability (clicking link will navigate to a different page, can right click to open in new window, copy link, etc.), and more simplicity from a design and code standpoint.

@Sarai-WMDE specifically regarding elements like the card or search result where the entire element is a link, I do understand your point and I can see it both ways. I don't think that because the entire thing is a link we shouldn't style the main text element within those elements as a link. I think you can find many examples on the internet pointing in either direction. Here are two examples I've found that demonstrate the approach I am proposing:

Jdlrobson added a subscriber: Jdlrobson.

Will take a look sometime this week (hoping for Wednesday or Thursday)

As discussed Jan will aim to merge this Tuesday 19th July so this change can go out with next week's train.

Change 758961 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Search: Use Codex and Vue 3 instead of WVUI and Vue 2.

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

Bug found in the CdxTypeaheadSearch component itself related to rendering of the no-results slot: T313335

Related minor styling issue: T313414

Change 804013 merged by jenkins-bot:

[mediawiki/extensions/CirrusSearch@master] article_page test: Change WVUI CSS selector to equivalent Codex selector

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

ovasileva renamed this task from Use CdxTypeaheadSearch in Vector to Typeahead search deployment: Use CdxTypeaheadSearch in Vector.Jul 21 2022, 5:00 PM
Edtadros added a subscriber: Edtadros.

Test Result - Beta

Status: Mixed
Environment: beta
OS: macOS Monterey
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

See steps here.

TC IDStatusDetails
T 1This is out of scope. Search button does not display on hover unless you have previously clicked into the field:
Screen Recording 2022-07-21 at 2.06.16 PM.mov.gif (404×808 px, 95 KB)
T 2
Screen Recording 2022-07-21 at 7.15.08 PM.mov.gif (404×808 px, 114 KB)
T 3
Screen Recording 2022-07-21 at 7.27.06 PM.mov.gif (476×898 px, 200 KB)
T 4
Screen Shot 2022-07-21 at 7.32.47 PM.png (1×1 px, 383 KB)
T 5
Screen Recording 2022-07-21 at 7.34.45 PM.mov.gif (630×548 px, 342 KB)
T 6
Screen Recording 2022-07-21 at 7.36.39 PM.mov.gif (710×784 px, 454 KB)
T 7
Screen Recording 2022-07-21 at 7.38.53 PM.mov.gif (710×784 px, 424 KB)
T 8
Screen Recording 2022-07-21 at 7.40.44 PM.mov.gif (710×866 px, 454 KB)
T 9
Screen Recording 2022-07-21 at 8.12.01 PM.mov.gif (802×1 px, 427 KB)
T 10
Screen Recording 2022-07-21 at 8.15.32 PM.mov.gif (802×1 px, 192 KB)
T 11
Screen Recording 2022-07-21 at 8.20.48 PM.mov.gif (802×1 px, 632 KB)
T 12A gap appears above the "Search for pages containing..." item.
Screen Recording 2022-07-21 at 8.26.09 PM.mov.gif (802×1 px, 427 KB)
T 13
Screen Recording 2022-07-21 at 8.27.38 PM.mov.gif (802×1 px, 313 KB)
T 14See T10.
T 15This is out of scope
T 16
Screen Shot 2022-07-21 at 8.43.00 PM.png (773×863 px, 160 KB)
T 17
Screen Shot 2022-07-21 at 8.45.06 PM.png (345×880 px, 76 KB)
T 18
Screen Recording 2022-07-21 at 8.47.22 PM.mov.gif (802×1 px, 410 KB)
T 19This is out of scope
Screen Recording 2022-07-21 at 8.39.00 PM.mov.gif (802×1 px, 1 MB)
T 20see T 18 above
T 21
Screen Shot 2022-07-25 at 7.54.52 PM.png (739×887 px, 148 KB)
T 22See T 21 above
T 23
Screen Shot 2022-07-25 at 7.57.12 PM.png (1×912 px, 206 KB)
T 24
Screen Shot 2022-07-25 at 7.58.50 PM.png (245×1 px, 38 KB)
T 25
Screen Recording 2022-07-25 at 8.06.25 PM.mov.gif (245×613 px, 1 MB)
T 26
Screen Recording 2022-07-25 at 8.12.45 PM.mov.gif (876×612 px, 166 KB)
this is a pass per conversation with @alexhollender_WMF

Let us know if you need any help from web in signing this off.