Skip to content
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.

Sign up for GitHub

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

Jump to bottom

LPS-139246 - Create custom autocomplete for object-dynamic-data-mapping #1471

Closed
wants to merge 9 commits into from
Closed

LPS-139246 - Create custom autocomplete for object-dynamic-data-mapping #1471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4e3cfaa
LPS-139246 - Create generic autocomplete to be used in Commerce and DDM
bryceosterhaus Sep 20, 2021
035ad2e
LPS-139246 - Update Commerce autocomplete to use generic one from fro…
bryceosterhaus Sep 20, 2021
7a209bf
LPS-139246 - Use generic autocomplete from frontend-js-components-web
bryceosterhaus Sep 20, 2021
1486d9a
LPS-139246 - Update data fetching logic to use provided query
bryceosterhaus Sep 21, 2021
2645b61
LPS-139246 - Fix commerce autocomplete to not be a separate drop down
bryceosterhaus Sep 21, 2021
691a59c
LPS-139246 - Remove unused component
bryceosterhaus Sep 21, 2021
48f33c2
LPS-139246 - Remove redundant value, we can use initialValue for this
bryceosterhaus Sep 21, 2021
edb68b5
LPS-139246 - Remove unnecessary autocomplete component
bryceosterhaus Sep 21, 2021
d211506
LPS-139246 - SF
bryceosterhaus Sep 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 0 additions & 65 deletions modules/apps/commerce/commerce-frontend-js/dev/components/Autocomplete.js
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions modules/apps/commerce/commerce-frontend-js/dev/components/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@ module.exports = [
name: 'Add To Wish List',
page: 'add-to-wish-list.html',
},
{
entry: 'Autocomplete',
name: 'Autocomplete',
page: 'autocomplete.html',
},
{
entry: 'DatasetDisplay',
name: 'Dataset display',
Expand Down
26 changes: 0 additions & 26 deletions modules/apps/commerce/commerce-frontend-js/dev/public/autocomplete.html
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions modules/apps/commerce/commerce-frontend-js/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"@clayui/tooltip": "3.35.3",
"@liferay/frontend-js-react-web": "*",
"classnames": "2.3.1",
"frontend-js-components-web": "*",
"frontend-js-web": "*",
"prop-types": "15.7.2",
"react-dom": "16.12.0"
Expand Down
25 changes: 15 additions & 10 deletions ...c/main/resources/META-INF/resources/components/account_selector/views/AccountsListView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

import {ClayButtonWithIcon} from '@clayui/button';
import ClayDropDown from '@clayui/drop-down';
import React, {useRef} from 'react';
import {ClayInput} from '@clayui/form';
import React from 'react';

import ServiceProvider from '../../../ServiceProvider/index';
import Sticker from '../Sticker';
Expand All @@ -32,7 +33,7 @@ function AccountsListView({
disabled,
setCurrentView,
}) {
const accountsListRef = useRef();
const [inputValue, setInputValue] = React.useState('');

return (
<ClayDropDown.ItemList className="accounts-list-container">
Expand All @@ -55,9 +56,19 @@ function AccountsListView({
<ClayDropDown.Divider />

<ClayDropDown.Section>
<ClayInput
disabled={disabled}
onChange={(event) => setInputValue(event.target.value)}
placeholder={Liferay.Language.get('search')}
value={inputValue}
/>
</ClayDropDown.Section>

<ClayDropDown.Divider />

<li>
<ListView
apiUrl={ACCOUNTS_RESOURCE_ENDPOINT}
contentWrapperRef={accountsListRef}
customView={({items, loading}) => {
if (!items || !items.length) {
return (
Expand Down Expand Up @@ -93,14 +104,8 @@ function AccountsListView({
);
}}
disabled={disabled}
placeholder={Liferay.Language.get('search')}
query={inputValue}
/>
</ClayDropDown.Section>

<ClayDropDown.Divider />

<li>
<div ref={accountsListRef} />
</li>
</ClayDropDown.ItemList>
);
Expand Down
78 changes: 57 additions & 21 deletions ...nd-js/src/main/resources/META-INF/resources/components/account_selector/views/ListView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,30 +12,66 @@
* details.
*/

import React from 'react';
import {useIsMounted} from '@liferay/frontend-js-react-web';
import React, {useEffect, useState} from 'react';

import Autocomplete from '../../autocomplete/Autocomplete';
import debounce from '../../../utilities/debounce';
import {getData} from '../../../utilities/index';
import {showErrorNotification} from '../../../utilities/notifications';
import InfiniteScroller from '../../infinite_scroller/InfiniteScroller';

function ListView({apiUrl, customView: CustomView, pageSize = 10, query}) {
const [items, setItems] = useState(null);
const [loading, setLoading] = useState(false);
const [totalCount, setTotalCount] = useState(null);
const [lastPage, setLastPage] = useState(null);
const [page, setPage] = useState(1);
const isMounted = useIsMounted();

const fetchData = debounce((queryVal) => {
if (queryVal && isMounted()) {
setLoading(true);

getData(apiUrl, queryVal, page, pageSize)
.then((jsonResponse) => {
setItems((prevItems) => {
if (prevItems?.length && page > 1) {
return [...prevItems, ...jsonResponse.items];
}

return jsonResponse.items;
});

setTotalCount(jsonResponse.totalCount);
setLastPage(jsonResponse.lastPage);
setLoading(false);
})
.catch(() => {
showErrorNotification();
setLoading(false);
});
}
}, 200);

useEffect(() => {
fetchData(query);
}, [fetchData, query]);

function ListView({
apiUrl,
contentWrapperRef,
customView,
disabled,
placeholder,
}) {
return (
<Autocomplete
apiUrl={apiUrl}
contentWrapperRef={contentWrapperRef}
customView={customView}
disabled={disabled}
infiniteScrollMode={true}
inputName={placeholder}
inputPlaceholder={Liferay.Language.get(placeholder)}
itemsKey="id"
itemsLabel="name"
pageSize={10}
/>
<InfiniteScroller
onBottomTouched={() => {
if (!loading) {
if (page !== lastPage) {
setPage((currentPage) => currentPage + 1);

fetchData(query);
}
}
}}
scrollCompleted={!items || items.length >= totalCount}
>
<CustomView items={items} loading={loading} />
</InfiniteScroller>
);
}

Expand Down
29 changes: 18 additions & 11 deletions ...src/main/resources/META-INF/resources/components/account_selector/views/OrdersListView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

import ClayButton, {ClayButtonWithIcon} from '@clayui/button';
import ClayDropDown from '@clayui/drop-down';
import React, {useMemo, useRef} from 'react';
import {ClayInput} from '@clayui/form';
import React, {useMemo} from 'react';

import ServiceProvider from '../../../ServiceProvider/index';
import {OPEN_MODAL} from '../../../utilities/eventsDefinitions';
Expand All @@ -34,13 +35,13 @@ function OrdersListView({
setCurrentView,
showOrderTypeModal,
}) {
const [inputValue, setInputValue] = React.useState('');

const CartResource = useMemo(
() => ServiceProvider.DeliveryCartAPI('v1'),
[]
);

const ordersListRef = useRef();

return (
<ClayDropDown.ItemList className="orders-list-container">
<ClayDropDown.Section className="item-list-head">
Expand All @@ -57,13 +58,23 @@ function OrdersListView({

<ClayDropDown.Divider />

<ClayDropDown.Section className="item-list-body">
<ClayDropDown.Section>
<ClayInput
disabled={disabled}
onChange={(event) => setInputValue(event.target.value)}
placeholder={Liferay.Language.get('search-order')}
value={inputValue}
/>
</ClayDropDown.Section>

<ClayDropDown.Divider />

<li>
<ListView
apiUrl={CartResource.cartsByAccountIdAndChannelIdURL(
currentAccount.id,
commerceChannelId
)}
contentWrapperRef={ordersListRef}
customView={({items, loading}) => {
if (!items || !items.length) {
return (
Expand All @@ -84,16 +95,12 @@ function OrdersListView({
);
}}
disabled={disabled}
placeholder={Liferay.Language.get('search-order')}
query={inputValue}
/>
</ClayDropDown.Section>
</li>

<ClayDropDown.Divider />

<li>
<div ref={ordersListRef} />
</li>
Comment on lines -93 to -95
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a very strange way to do this. I need to properly test this in the UI but I really don't like this pattern being used. Effectively this was an anti-pattern here of bi-directional data flow. If the Autocomplete component was passed a customView prop, instead of rendering inside of the Autocomplete component it was handling all the business logic of requesting items and then rendering them via createPortal to this node. The reason this pattern is bad is because Autocomplete is now rendering to an unrelated area. If we want to properly do this, we must pull the state(items) up the react tree and outside of Autocomplete. That way we either pass the items to Autocomplete or we render them here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @FabioDiegoMastrorilli

I noticed you added this API in liferay@4aa3853#diff-fbc58da25a23f789ae15c90776dd7b5ce184557ea3d17369a143dfb8bb7554bcR270

Can you verify my changes in this PR and see how we can get around this pattern of using the portal to render the content on an outside node.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @bryceosterhaus it makes totally sense :)
Passing a ref there would help in case we would like to use all the autocomplete functionalities (search, infinite scrolling, custom views) to render a list of results even out of a react context. In that case there won't be any state tree so it wont't look like an unsafe operation.

That being said... please have a look at the AccountSelector component. I've use the Autocomplete there to avoid code duplication and, if I could still use it there to get the same result, it would be great.


<ClayDropDown.Section>
<ClayButton
className="m-auto w-100"
Expand Down
Loading