CHK-134: Drop-down field style of Location Search autocomplete component #21

wdsrocha · 2020-07-16T20:45:27Z

What problem is this solving?

Continuation of #17 and #20, implements the styling part of the following component.

How should this be manually tested?

Add any product to cart;
Go to shipping step, it will be the first component displayed (it's placed there just for convenience).
Compare to Figma, play around, etc.

Usage example

import GoogleLogo from './../assets/powered_by_google.png'

// ...

<LocationSearch
  renderEngineLogo={() => (
    <img className="h1" src={GoogleLogo} alt="Powered by Google" />
  )}
/>

vtex-io-ci-cd · 2020-07-16T20:45:28Z

Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖

Please select which version do you want to release:

Patch (backwards-compatible bug fixes)
Minor (backwards-compatible functionality)
Major (incompatible API changes)

And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.

No thanks, I would rather do it manually 😞

vtex-io-docs-bot · 2020-07-16T20:45:29Z

Beep boop 🤖

I noticed you didn't make any changes at the docs/ folder

There's nothing new to document 🤔
I'll do it later 😞

In order to keep track, I'll create an issue if you decide now is not a good time

I just updated 🎉🎉

Notes: - there is no #DBE9FD as common bg color on Tachyons - used bg-action-secondary for aria-selected (#EEF3F7) instead

react/LocationSearch.tsx

react/addresses.tsx

react/LocationSearch.tsx

react/components/Combobox.tsx

wdsrocha · 2020-07-28T15:33:14Z

react/LocationSearch.tsx

+  const { mainText } = suggestion
+  const { offset, size } = suggestion.mainTextMatchInterval
+  return (
+    <div className="truncate c-muted-2">


When the text inside an Option is too big, it will truncate with an ellipsis, but there is a problem: the mainText (street name) is black and the secondaryText (city and state name) is gray, which would require the ellipsis color to be dynamic, depending on which part have overflowed.

Doing this at implementation level is quite painful and don't seems to bring too much value (big street names aren't very common, I guess), so I've opted to always display the ellipsis with a gray color. Is that okay? @davicosta99 @augustocb

Example of fake big street name with the grey colored ellipsis:

I think it's fine, Wesley, thanks for the concern. Appreciate the level of care 😉

wdsrocha · 2020-07-28T15:39:21Z

react/components/Combobox.css

+.option:active {
+  background-color: #d2defc
+}


@davicosta99, while addressing the following issue

When a suggestion is selected (on:active) it should blink in the on-action color token (#D2DEFC) for 50ms before closing.

I found out that this color does not exist on VTEX Tachyons spec, so I’ve added it directly here.

Also, how do I know if it takes 50ms during the blink? Is that the standard behavior?

Same goes for #c4c4c4 on Place Icon and #2e2e2e on Warning Icon, but I used the closest "c-muted-x" for them:

#c4c4c4 -> #cacbcc (c-muted-3)

#2e2e2e -> #727273 (c-muted-1)

Currently we have these tokens, that basically reuse the hover color.

.active-bg-action-secondary:active { background-color: #dbe9fd; } .active-b--action-secondary:active { border-color: #dbe9fd; }

Can you change them to #D2DEFC? This change was made in Figma but never implemented.

Regarding the 50ms, I thought this was something that needed to be defined. Right now is perfect already.

Sorry for the mistake on the grays, my bad. Thanks for using the right tokens 😉

wdsrocha · 2020-07-28T19:17:48Z

@davicosta99

The max-width for the field is not the same as in the Figma file.

This task/PR only addresses the LocationSearch component itself, so I'll worry with the resize later on, when integrating with the NewFormAddress component inside the shipping step. But thanks for pointing that out, I ended up finding a bug related to that!

I'm not able to test on my phone because after submitting the profile step I get this error

I don't know why this is happening to you, just tried on my phone and it seems to be working fine. Could you try again, starting from the cart step?

wdsrocha · 2020-07-28T19:32:22Z

react/LocationSearch.tsx

+interface Interval {
+  offset: number
+  size: number
+}

-const MAX_DROPDOWN_ADDRESSES = 6
+export interface Suggestion {
+  description: string
+  mainText: string
+  mainTextMatchInterval: Interval
+  secondaryText: string
+}

-// This function will be replaced in the future, after integrating the
-// component with GraphQL queries.
-const getAddresses = (searchTerm: string) => {
-  return mockedAddresses
-    .filter((address: string) =>
-      address.toLocaleLowerCase().includes(searchTerm.toLocaleLowerCase())
-    )
-    .slice(0, MAX_DROPDOWN_ADDRESSES)
+const renderSuggestionText = (suggestion: Suggestion) => {
+  const { mainText } = suggestion
+  const { offset, size } = suggestion.mainTextMatchInterval
+  return (
+    <div className="truncate c-muted-2">
+      <span className="c-on-base">{mainText.substr(0, offset)}</span>
+      <span className="c-on-base b">{mainText.substr(offset, size)}</span>
+      <span className="c-on-base">{mainText.substr(size + offset)}</span>
+      <span>{` ${suggestion.secondaryText}`}</span>
+    </div>
+  )
 }


About the full address being displayed

In order to handle this, I’ve opted for implementing something similar to Google autocomplete (see AutocompletePrediction Interface). When the user inputs the text "bri" into the input box, an answer like this will be returned:

[ { "description": "Cambridge, Rockford, Illinois", "mainText": "Cambridge", "secondaryText": "Rockford, Illinois", "mainTextMatchInterval": { "offset": 3, "size": 3 } }, { "description": "Briar Crest, Hagerstown, Maryland", "mainText": "Briar Crest", "secondaryText": "Hagerstown, Maryland", "mainTextMatchInterval": { "offset": 0, "size": 3 } } ]

Which will be converted to (just the first one, for simplicity):

<div className="truncate c-muted-2"> Cam bri dge Rockford, Illinois </div>

And displayed like:

Explanation of the fields:

description: the text that will go to the input box when the user selects it

mainText: probably the street name (along with the number, maybe?)

secondaryText: larger administrative regions like city, state and country

mainTextMatchInterval: used to highlight the part of the mainText that matched the user input. This can be improved to hold multiple matches and/or include the secondaryText, but for now, I think imitating the Google behavior might be sufficient (which is just like that).

This isn't very flexible, but looks sufficient to me. Opinions?

Seems enough for me as well. I also don't believe we should include the secondaryText in the mainTextMatchInterval or multiple matches.

davicostalf

Besides including the color in our Tachyons like I've mentioned above, the only other issue I found was this:

It seems like no result is found when the secondary text is included in the search term. Is this happening because of the mocked data or does something need to be corrected?

lucasecdb · 2020-07-29T20:01:33Z

react/LocationSearch.tsx

+      <span className="c-on-base">{mainText.substr(0, offset)}</span>
+      <span className="c-on-base b">{mainText.substr(offset, size)}</span>
+      <span className="c-on-base">{mainText.substr(size + offset)}</span>


can we opt in to a simpler markup? like the following

Suggested change

{mainText.substr(0, offset)}

{mainText.substr(offset, size)}

{mainText.substr(size + offset)}



{mainText.substr(0, offset)}

{mainText.substr(offset, size)}

{mainText.substr(size + offset)}

I think so, it looks better indeed. I was just worried that this wouldn't be good at a semantic level, but thinking again, I guess there is nothing wrong with a nested span.

yep, it's an inline element inside another inline element, so I think we should be fine

wdsrocha · 2020-07-29T20:24:08Z

It seems like no result is found when the secondary text is included in the search term. Is this happening because of the mocked data or does something need to be corrected?

@davicosta99 as commented here, the current search algorithm is pretty naive and for testing purposes only. But I'll keep an eye on caching, as I suppose it does not make too much sense to re-do a request for this kind of search that you have pointed out.

lucasecdb

LGTM, left a nitpick

lucasecdb · 2020-07-30T12:19:08Z

react/LocationSearch.tsx

+        <span className="b">{mainText.substr(offset, size)}</span>
+        {mainText.substr(size + offset)}
+      </span>
+      <span>{` ${suggestion.secondaryText}`}</span>


sorry I missed this in the last review, but it's just a nitpick

Suggested change

{` ${suggestion.secondaryText}`}

{' '}{suggestion.secondaryText}

Done, but the formatter sent the whitespace to the line above.

wdsrocha · 2020-07-30T19:22:49Z

react/components/Combobox.tsx

@@ -55,7 +55,7 @@ export const ComboboxOption: React.FC<ReachComboboxOptionProps & {
    className={classNames(
      className,
      styles.option,
-      'flex items-center pointer bg-action-secondary pv3 ph5 hover-bg-action-secondary'
+      'flex items-center pv3 ph5 pointer bg-action-secondary hover-bg-action-secondary active-bg-action-secondary'


Now this is "incorrect", but as soon as Styleguide bumps the Tachyons version, I'll open a PR bumping the Styleguide version here and it will have the expected color.

you don't actually need to bump styleguide here after deploying the new version with vtex-tachyons updated, because the IO infra will automatically bump the styleguide version on the account itself when it is published.

davicostalf

LGTM

wdsrocha added 5 commits July 16, 2020 18:57

Add popover style and decouple component

1ed6afd

Add list style

965707d

Add kind of broken Option

77ae1f5

Fix popover style

cb9a8c2

Fix option alignment a little bit

1b57356

wdsrocha force-pushed the feature/combobox-style branch from 088d57b to 1b57356 Compare July 16, 2020 21:59

wdsrocha added 6 commits July 21, 2020 14:52

Put drop-down in the right place

afa5148

Use symetric padding on Option and fix icon shrink

a643966

Add styling on msg displayed when the search fails

fe3e4fa

Remove redundant truncate and make fail msg grey

3bc5d5c

Add big addresses so we can test text overflow

dae152f

Shortnen spanish text so it fits the component (gracias Maxito 🇦🇷)

1e11bbe

wdsrocha force-pushed the feature/combobox-style branch from 9877269 to 1e11bbe Compare July 21, 2020 19:15

wdsrocha added 3 commits July 21, 2020 16:35

Add Option bg color on hover

88b062b

Notes: - there is no #DBE9FD as common bg color on Tachyons - used bg-action-secondary for aria-selected (#EEF3F7) instead

Add mocked Google logo

7127d88

Add prop for engine logo

abe1326

wdsrocha commented Jul 22, 2020

View reviewed changes

react/LocationSearch.tsx Outdated Show resolved Hide resolved

wdsrocha commented Jul 22, 2020

View reviewed changes

react/addresses.tsx Outdated Show resolved Hide resolved

wdsrocha marked this pull request as ready for review July 22, 2020 15:20

wdsrocha requested a review from a team as a code owner July 22, 2020 15:20

wdsrocha requested review from darlenedms, jeffersontuc and lucasecdb and removed request for a team July 22, 2020 15:20

lucasecdb reviewed Jul 22, 2020

View reviewed changes

wdsrocha added 3 commits July 23, 2020 13:58

Refactor ref variable name

a013635

Document why we clear button isn't tabbable

72e205d

Stop sending props to renderEngineLogo

5156127

wdsrocha added 3 commits July 24, 2020 19:44

Fix Place Icon color

3ad803d

Fix clear icon color on hover

734f0fc

Fix option color when active

e44fe25

wdsrocha commented Jul 28, 2020

View reviewed changes

wdsrocha added 3 commits July 28, 2020 12:22

Fix color and text width from search failure

e1fdd9a

Increase Clear button clickable area

656efdc

Let component width be handled by parent

a2caaa2

wdsrocha requested a review from augustocb July 28, 2020 19:18

wdsrocha commented Jul 28, 2020

View reviewed changes

wdsrocha requested a review from lucasecdb July 28, 2020 19:36

davicostalf requested changes Jul 29, 2020

View reviewed changes

lucasecdb reviewed Jul 29, 2020

View reviewed changes

Simplify renderSuggestionText

9823bf7

wdsrocha mentioned this pull request Jul 29, 2020

Fix active action-secondary bg and border colors vtex/vtex-tachyons#32

Merged

lucasecdb approved these changes Jul 30, 2020

View reviewed changes

wdsrocha mentioned this pull request Jul 30, 2020

Fix active action-secondary bg and border colors vtex-apps/store-theme#224

Merged

wdsrocha added 3 commits July 30, 2020 14:55

Remove unused abstraction

94ba39f

Use styling from Tachyons

2c1428d

Refactor nit

0bfa9b3

wdsrocha commented Jul 30, 2020

View reviewed changes

lucasecdb approved these changes Jul 30, 2020

View reviewed changes

wdsrocha requested a review from davicostalf July 31, 2020 13:08

davicostalf approved these changes Jul 31, 2020

View reviewed changes

wdsrocha changed the title ~~CHK-134: Drop-down field style of Location Search autocomplete component~~ Refactor and style Location Search autocomplete Jul 31, 2020

wdsrocha merged commit e184d06 into feature/location-search Jul 31, 2020

wdsrocha deleted the feature/combobox-style branch July 31, 2020 22:12

wdsrocha changed the title ~~Refactor and style Location Search autocomplete~~ CHK-134: Drop-down field style of Location Search autocomplete component Jul 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CHK-134: Drop-down field style of Location Search autocomplete component #21

CHK-134: Drop-down field style of Location Search autocomplete component #21

wdsrocha commented Jul 16, 2020 •

edited

Loading

vtex-io-ci-cd bot commented Jul 16, 2020 •

edited by wdsrocha

Loading

vtex-io-docs-bot bot commented Jul 16, 2020 •

edited by wdsrocha

Loading

wdsrocha Jul 28, 2020 •

edited

Loading

davicostalf Jul 29, 2020

wdsrocha Jul 28, 2020

wdsrocha Jul 28, 2020

davicostalf Jul 29, 2020 •

edited

Loading

wdsrocha commented Jul 28, 2020

wdsrocha Jul 28, 2020

davicostalf Jul 29, 2020 •

edited

Loading

davicostalf left a comment •

edited

Loading

lucasecdb Jul 29, 2020

wdsrocha Jul 29, 2020

lucasecdb Jul 29, 2020

wdsrocha commented Jul 29, 2020

lucasecdb left a comment

lucasecdb Jul 30, 2020

wdsrocha Jul 30, 2020

wdsrocha Jul 30, 2020

lucasecdb Jul 30, 2020

davicostalf left a comment

	<span>{` ${suggestion.secondaryText}`}</span>
	{' '}<span>{suggestion.secondaryText}</span>

CHK-134: Drop-down field style of Location Search autocomplete component #21

CHK-134: Drop-down field style of Location Search autocomplete component #21

Conversation

wdsrocha commented Jul 16, 2020 • edited Loading

What problem is this solving?

How should this be manually tested?

Usage example

vtex-io-ci-cd bot commented Jul 16, 2020 • edited by wdsrocha Loading

vtex-io-docs-bot bot commented Jul 16, 2020 • edited by wdsrocha Loading

wdsrocha Jul 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davicostalf Jul 29, 2020 • edited Loading

Choose a reason for hiding this comment

wdsrocha commented Jul 28, 2020

Choose a reason for hiding this comment

About the full address being displayed

davicostalf Jul 29, 2020 • edited Loading

Choose a reason for hiding this comment

davicostalf left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wdsrocha commented Jul 29, 2020

lucasecdb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davicostalf left a comment

Choose a reason for hiding this comment

wdsrocha commented Jul 16, 2020 •

edited

Loading

vtex-io-ci-cd bot commented Jul 16, 2020 •

edited by wdsrocha

Loading

vtex-io-docs-bot bot commented Jul 16, 2020 •

edited by wdsrocha

Loading

wdsrocha Jul 28, 2020 •

edited

Loading

davicostalf Jul 29, 2020 •

edited

Loading

davicostalf Jul 29, 2020 •

edited

Loading

davicostalf left a comment •

edited

Loading