Page MenuHomePhabricator

Style search widget results based on project configuration
Closed, ResolvedPublic3 Estimated Story Points

Description

Background

In T260167: Make new search widget configurable per project we are allowing the search results to be configured so that they display the information most relevant to a given wiki. This will require a change in the design of the search results. This task is about the new design

Acceptance criteria

  • Style the widget based on the design requirements below

Design

description + imageno descriptionno imageno image + no description
Screen Shot 2020-09-29 at 2.21.47 PM.png (653×788 px, 163 KB)
Screen Shot 2020-09-28 at 4.20.23 PM.png (666×804 px, 124 KB)
Screen Shot 2020-09-28 at 4.25.31 PM.png (639×802 px, 135 KB)
Screen Shot 2020-09-28 at 4.25.57 PM.png (482×802 px, 74 KB)
QA linkQA linkQA linkQA link

Prototype: https://di-search-3.web.app/Volvo

QA

  • Use the QA links in the table above and verify the screenshots match what is seen.
  • If a QA link is not provided when you need it, please ask in Slack :)

QA Results - Patchdemo

Event Timeline

ovasileva created this task.

Change 637776 had a related patch set uploaded (by Nray; owner: Nray):
[wvui@master] [components][typeahead-search][typeahead-suggestion] Style showThumbnail prop

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

@alexhollender Should the input still expand upon focus if we configure thumbnails to NOT be shown?

@alexhollender Should the input still expand upon focus if we configure thumbnails to NOT be shown?

No. In that case of no thumbnails the input would not expand. You can configure this and play with it in the demo

image.png (760×1 px, 308 KB)

Change 638212 had a related patch set uploaded (by Nray; owner: Nray):
[wvui@master] [typeahead-search][typeahead-suggestion] Unify figure size and spacing between figure and text

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

https://gerrit.wikimedia.org/r/c/wvui/+/638212 is ready for review. I'm hoping that will chip away a big chunk of this ticket

Change 638212 merged by jenkins-bot:
[wvui@master] [typeahead-search][typeahead-suggestion] Unify figure size and spacing between figure and text

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

nray removed nray as the assignee of this task.Nov 6 2020, 8:43 PM
nray added a subscriber: nray.

Change 637776 merged by jenkins-bot:
[wvui@master] [components][typeahead-search][typeahead-suggestion] Style showThumbnail prop

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

@alexhollender the style changes have been merged now. Can you take a look at https://wvui.netlify.app/ui/?path=/story/components-typeaheadsearch--typeahead-search and review?

You can use the "Show thumbnail?" and "Show description?" knobs to adjust the config

Note: there was a regression that was merged causing the suggestion list to be wider (the full width of the search component) than it should be, but that is a separate issue not relevant to this ticket

storybook.png (1×2 px, 305 KB)

@nray sorry for the delay — everything looks great.

mockstorybook
description + image
Screen Shot 2020-09-29 at 2.21.47 PM.png (653×788 px, 163 KB)
Screen Shot 2020-11-30 at 11.59.00 AM.png (618×549 px, 107 KB)
no description
Screen Shot 2020-09-28 at 4.20.23 PM.png (666×804 px, 124 KB)
Screen Shot 2020-11-30 at 11.59.17 AM.png (617×547 px, 66 KB)
no image
Screen Shot 2020-09-28 at 4.25.31 PM.png (639×802 px, 135 KB)
Screen Shot 2020-11-30 at 11.59.40 AM.png (599×523 px, 76 KB)
no image + no description
Screen Shot 2020-09-28 at 4.25.57 PM.png (482×802 px, 74 KB)
Screen Shot 2020-11-30 at 11.59.56 AM.png (453×516 px, 33 KB)

Change 643368 had a related patch set uploaded (by Nray; owner: Nray):
[wvui@master] [typeahead-suggestion] Replace min-width with flex-shrink

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

Change 644390 had a related patch set uploaded (by Nray; owner: Nray):
[wvui@master] [POC][typeahead-suggestion] Use calc for calculations involving em and px units

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

Change 644570 had a related patch set uploaded (by Nray; owner: Nray):
[wvui@master] [typeahead-search] Drop @min-width-typeahead-suggestion-thumb/use px for @size-typeahead-suggestion-thumb

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

Thanks for the review @alexhollender

An update on this task: There are two more patches related to this task that I am seeking to get merged before I move this ticket to sign off. The remaining patches try to solve alignment issues that surface when WVUI is placed in Vector (the different font-size used in Vector vs Storybook makes it apparent):

Replace min-width with flex-shrink

and either

a) Drop @min-width-typeahead-suggestion-thumb/use px for @size-typeahead-suggestion-thumb

OR

b) Use calc for calculations involving em and px units

@Volker_E has raised concerns about option "a" because it prevents the thumbnail size from being adjustable via the browser's font size setting. Option "b" circumvents this issue but seems like it is more complexity than we can afford right now and makes me favor option "a" at least for v1 of the search component.

nray reassigned this task from nray to Volker_E.
nray reassigned this task from Volker_E to Jdrewniak.

Blah Sorry for all the reassigns lol. Meant to assign to Jan as he said he wanted to take a look in standup today

Change 643368 merged by jenkins-bot:
[wvui@master] [typeahead-suggestion] Replace min-width with flex-shrink

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

I like the calc solution because it scales better with different font-sizes.
This is not necessarily a big issue for accessibility (text still scales) but I think it will make the typeahead usable in more contexts, like across skins with different font-sizes or when a smaller or larger typeahead is needed. So IMO, I think the added complexity of calc is worth it in this case.

Screen Shot 2020-12-03 at 12.36.55 PM.png (298×882 px, 48 KB)

Screen Shot 2020-12-03 at 12.38.31 PM.png (314×880 px, 153 KB)

Change 644570 abandoned by Nray:
[wvui@master] [typeahead-search] Drop @min-width-typeahead-suggestion-thumb/use px for @size-typeahead-suggestion-thumb

Reason:
Both Volker and Jan have expressed interest in using not using px units here. Please see I256490ae85c9b004441a1a48cf8b256c76e92508 for an alternative solution using calc

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

Change 645409 had a related patch set uploaded (by Nray; owner: Nray):
[wvui@master] [typeahead-search] Correct @size-typeahead-search-focus-addition value

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

Change 644390 merged by jenkins-bot:
[wvui@master] [typeahead-search] Use calc for calculations involving em and px units

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

Change 645409 merged by jenkins-bot:
[wvui@master] [typeahead-search] Correct @size-typeahead-search-focus-addition value

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

Edtadros added a subscriber: Edtadros.

Test Result - Patchdemo

Status: ✅ Pass
Environment: Patchdemo
OS: macOS Catalina
Browser: Chrome
Device: MBP
Emulated Device: NA

Test Artifact(s):

QA Steps
AC✅ AC1: description + image✅ AC2: no description✅ AC3: no image✅ AC4: no image + no description
Expected
Screen Shot 2020-09-29 at 2.21.47 PM.png (653×788 px, 163 KB)
Screen Shot 2020-09-28 at 4.20.23 PM.png (666×804 px, 124 KB)
Screen Shot 2020-09-28 at 4.25.31 PM.png (639×802 px, 135 KB)
Screen Shot 2020-09-28 at 4.25.57 PM.png (482×802 px, 74 KB)
LinkQA linkQA linkQA linkQA link
Actual
Screen Shot 2020-12-10 at 9.51.56 AM.png (718×792 px, 132 KB)
Screen Shot 2020-12-10 at 9.52.40 AM.png (718×831 px, 99 KB)
Screen Shot 2020-12-10 at 12.10.51 PM.png (674×795 px, 94 KB)
Screen Shot 2020-12-10 at 9.53.29 AM.png (574×799 px, 64 KB)

@Jdlrobson The search results look good, the only issue I see is the bottom entry that just says "containing" instead of "Search for pages containing". Let me know if this is part of the acceptance criteria or just the search results that appear in the drop-down are to be considered.

Update: The bottom entry is okay per T263657#6683760

Also noted in T249299#6681030 so I think we can pass this one for now and cover that in that ticket.