-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CHK-134: Drop-down field style of Location Search autocomplete component #21
Changes from 29 commits
1ed6afd
965707d
77ae1f5
cb9a8c2
1b57356
afa5148
a643966
fe3e4fa
3bc5d5c
dae152f
1e11bbe
88b062b
7127d88
abe1326
a013635
72e205d
5156127
d2c3da5
4b3edac
80b5967
bb0ae9d
c562b3b
3ad803d
734f0fc
e44fe25
e1fdd9a
656efdc
a2caaa2
9823bf7
94ba39f
2c1428d
0bfa9b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,37 +1,61 @@ | ||||||
import React, { useState, useMemo } from 'react' | ||||||
import React, { useState, useMemo, useRef } from 'react' | ||||||
import { FormattedMessage } from 'react-intl' | ||||||
import { Input, IconSearch, IconClear } from 'vtex.styleguide' | ||||||
import { Input, IconSearch, IconClear, IconWarning } from 'vtex.styleguide' | ||||||
import { positionMatchWidth } from '@reach/popover' | ||||||
|
||||||
import { | ||||||
Combobox, | ||||||
ComboboxInput, | ||||||
ComboboxOption, | ||||||
ComboboxPopover, | ||||||
ComboboxList, | ||||||
} from '@reach/combobox' | ||||||
import '@reach/combobox/styles.css' | ||||||
} from './components/Combobox' | ||||||
import PlaceIcon from './components/PlaceIcon' | ||||||
|
||||||
import { addresses as mockedAddresses } from './addresses' | ||||||
import styles from './LocationSearch.css' | ||||||
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 className="b">{mainText.substr(offset, size)}</span> | ||||||
{mainText.substr(size + offset)} | ||||||
</span> | ||||||
<span>{` ${suggestion.secondaryText}`}</span> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry I missed this in the last review, but it's just a nitpick
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, but the formatter sent the whitespace to the line above. |
||||||
</div> | ||||||
) | ||||||
} | ||||||
|
||||||
interface LocationSearchProps { | ||||||
getAddresses: (searchTerm: string) => Suggestion[] | ||||||
onSelectAddress?: (selectedAddress: string) => void | ||||||
renderEngineLogo?: () => React.ReactNode | ||||||
} | ||||||
|
||||||
const LocationSearch: React.FC<LocationSearchProps> = ({ onSelectAddress }) => { | ||||||
const LocationSearch: React.FC<LocationSearchProps> = ({ | ||||||
getAddresses, | ||||||
onSelectAddress, | ||||||
renderEngineLogo, | ||||||
}) => { | ||||||
const [searchTerm, setSearchTerm] = useState<string>('') | ||||||
const addresses = useMemo(() => getAddresses(searchTerm), [searchTerm]) | ||||||
const addresses = useMemo(() => getAddresses(searchTerm), [ | ||||||
getAddresses, | ||||||
searchTerm, | ||||||
]) | ||||||
const inputWrapperRef = useRef<HTMLDivElement>(null) | ||||||
|
||||||
const handleKeyDown = (event: React.KeyboardEvent) => { | ||||||
if (event.key !== 'Escape') { | ||||||
|
@@ -46,48 +70,76 @@ const LocationSearch: React.FC<LocationSearchProps> = ({ onSelectAddress }) => { | |||||
} | ||||||
|
||||||
return ( | ||||||
<div className={`${styles.locationSearch} w-100`}> | ||||||
<div className="w-100"> | ||||||
<Combobox onSelect={handleAddressSelection}> | ||||||
<ComboboxInput | ||||||
as={Input} | ||||||
testId="location-search-input" | ||||||
label={ | ||||||
<FormattedMessage id="place-components.label.autocompleteAddress" /> | ||||||
} | ||||||
prefix={ | ||||||
<div className="c-action-primary flex justify-center items-center"> | ||||||
<IconSearch /> | ||||||
</div> | ||||||
} | ||||||
suffix={ | ||||||
searchTerm.trim().length && ( | ||||||
<span | ||||||
data-testid="location-search-clear" | ||||||
role="button" | ||||||
tabIndex={-1} | ||||||
className="pointer c-muted-3 flex justify-center items-center outline-0" | ||||||
onClick={() => setSearchTerm('')} | ||||||
onKeyPress={() => {}} | ||||||
> | ||||||
<IconClear /> | ||||||
</span> | ||||||
<div ref={inputWrapperRef}> | ||||||
<ComboboxInput | ||||||
as={Input} | ||||||
testId="location-search-input" | ||||||
label={ | ||||||
<FormattedMessage id="place-components.label.autocompleteAddress" /> | ||||||
} | ||||||
prefix={ | ||||||
<div className="c-action-primary flex justify-center items-center"> | ||||||
<IconSearch /> | ||||||
</div> | ||||||
} | ||||||
suffix={ | ||||||
searchTerm.trim().length && ( | ||||||
<span | ||||||
data-testid="location-search-clear" | ||||||
role="button" | ||||||
// the input can be cleared by pressing the esc key, | ||||||
// so the clear button should not be tabbable | ||||||
tabIndex={-1} | ||||||
lucasecdb marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
className="flex pa3 na3 pointer outline-0 c-muted-3 hover-gray" | ||||||
onClick={() => setSearchTerm('')} | ||||||
onKeyPress={() => {}} | ||||||
> | ||||||
<IconClear /> | ||||||
</span> | ||||||
) | ||||||
} | ||||||
value={searchTerm} | ||||||
onChange={(event: React.ChangeEvent<HTMLInputElement>) => | ||||||
setSearchTerm(event.target.value) | ||||||
} | ||||||
onKeyDown={handleKeyDown} | ||||||
/> | ||||||
</div> | ||||||
<ComboboxPopover | ||||||
position={(_targetRect, popoverRect) => | ||||||
positionMatchWidth( | ||||||
inputWrapperRef.current?.getBoundingClientRect(), | ||||||
popoverRect | ||||||
) | ||||||
} | ||||||
value={searchTerm} | ||||||
onChange={(event: React.ChangeEvent<HTMLInputElement>) => | ||||||
setSearchTerm(event.target.value) | ||||||
} | ||||||
onKeyDown={handleKeyDown} | ||||||
/> | ||||||
<ComboboxPopover> | ||||||
> | ||||||
{addresses.length > 0 ? ( | ||||||
<ComboboxList> | ||||||
{addresses.map((address, index) => ( | ||||||
<ComboboxOption value={address} key={index} /> | ||||||
))} | ||||||
</ComboboxList> | ||||||
<> | ||||||
<ComboboxList> | ||||||
{addresses.map((address, index) => ( | ||||||
<ComboboxOption value={address.description} key={index}> | ||||||
<PlaceIcon className="flex flex-shrink-0 mr4 c-muted-1" /> | ||||||
{renderSuggestionText(address)} | ||||||
</ComboboxOption> | ||||||
))} | ||||||
</ComboboxList> | ||||||
{renderEngineLogo && ( | ||||||
<div className="flex flex-row-reverse"> | ||||||
<div className="mt3 mb1 mh5">{renderEngineLogo()}</div> | ||||||
</div> | ||||||
)} | ||||||
</> | ||||||
) : ( | ||||||
<FormattedMessage id="place-components.label.autocompleteAddressFail" /> | ||||||
<div className="flex items-center pv3 ph5"> | ||||||
<div className="flex flex-shrink-0 mr4 c-muted-3"> | ||||||
<IconWarning /> | ||||||
</div> | ||||||
<div className="truncate c-muted-2 fw6"> | ||||||
<FormattedMessage id="place-components.label.autocompleteAddressFail" /> | ||||||
</div> | ||||||
</div> | ||||||
)} | ||||||
</ComboboxPopover> | ||||||
</Combobox> | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 thesecondaryText
(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:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine, Wesley, thanks for the concern. Appreciate the level of care 😉