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.

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

Replace Select with Autocomplete with single Selects #1212

Merged
merged 13 commits into from
Mar 8, 2024
Merged
59 changes: 41 additions & 18 deletions modern/src/common/components/SelectField.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {
FormControl, InputLabel, MenuItem, Select,
FormControl, InputLabel, MenuItem, Select, Autocomplete, TextField
} from '@mui/material';
import React, { useState } from 'react';
import { useEffectAsync } from '../../reactHelper';
Expand All @@ -8,9 +8,9 @@ const SelectField = ({
label,
fullWidth,
multiple,
value,
emptyValue = 0,
emptyTitle = '\u00a0',
jinzo marked this conversation as resolved.
Show resolved Hide resolved
value = null,
emptyValue = null,
emptyTitle = '',
onChange,
endpoint,
data,
Expand All @@ -19,6 +19,13 @@ const SelectField = ({
}) => {
const [items, setItems] = useState(data);

const getOptionLabel = (option) => {
if (typeof option !== 'object') {
option = items.find(obj => keyGetter(obj) === option);
}
return option ? titleGetter(option) : emptyTitle;
}

useEffectAsync(async () => {
if (endpoint) {
const response = await fetch(endpoint);
Expand All @@ -33,20 +40,36 @@ const SelectField = ({
if (items) {
return (
<FormControl fullWidth={fullWidth}>
<InputLabel>{label}</InputLabel>
<Select
label={label}
multiple={multiple}
value={value}
onChange={onChange}
>
{!multiple && emptyValue !== null && (
<MenuItem value={emptyValue}>{emptyTitle}</MenuItem>
)}
{items.map((item) => (
<MenuItem key={keyGetter(item)} value={keyGetter(item)}>{titleGetter(item)}</MenuItem>
))}
</Select>
{multiple ? (
Copy link
Member

Choose a reason for hiding this comment

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

I think overall looks good. But I forgot now why are we not using Autocomplete for multi-select? It seems like it would be the most valuable there actually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I agree, but I did not find an easy way to make it work/look 'properly' - that's why I split it up in two and first implemented single select.
The problem is that the MUI Autocomplete component when using multiple values 'adds' another row in the Input field and that does not look that good - see this comment for the image: #1189 (comment)
The idea was to move the actual 'search input' into the drop down list instead of growing the original input to 2 lines (so the original input would only contain selected values), like select2 does for example - but I did not find a sane/easy way of doing that at the time. So the decision was made to first do it for single select, get that merged so it makes my life easier with my big lists, then tackle the multi usecase later.

<>
<InputLabel>{label}</InputLabel>
<Select
label={label}
multiple
value={value}
onChange={onChange}
>
{items.map((item) => (
<MenuItem key={keyGetter(item)} value={keyGetter(item)}>{titleGetter(item)}</MenuItem>
))}
</Select>
</>
) : (
<>
jinzo marked this conversation as resolved.
Show resolved Hide resolved
<Autocomplete
size="small"
options={items}
getOptionLabel={getOptionLabel}
renderOption={(props, option) => (
<MenuItem {...props} key={keyGetter(option)} value={keyGetter(option)}>{titleGetter(option)}</MenuItem>
)}
isOptionEqualToValue={(option, value) => keyGetter(option) == value}
value={value}
onChange={(e, nV) => {onChange({target:{value:nV ? keyGetter(nV) : emptyValue}})}}
jinzo marked this conversation as resolved.
Show resolved Hide resolved
renderInput={(params) => <TextField {...params} label={label} />}
/>
</>
)}
</FormControl>
);
}
Expand Down
46 changes: 18 additions & 28 deletions modern/src/reports/components/ReportFilter.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { useState } from 'react';
import {
FormControl, InputLabel, Select, MenuItem, Button, TextField, Typography,
FormControl, InputLabel, Select, MenuItem, Button, TextField, Typography, Autocomplete
} from '@mui/material';
import { useDispatch, useSelector } from 'react-redux';
import dayjs from 'dayjs';
Expand Down Expand Up @@ -92,36 +92,26 @@ const ReportFilter = ({ children, handleSubmit, handleSchedule, showOnly, ignore
<div className={classes.filter}>
{!ignoreDevice && (
<div className={classes.filterItem}>
<FormControl fullWidth>
<InputLabel>{t(multiDevice ? 'deviceTitle' : 'reportDevice')}</InputLabel>
<Select
label={t(multiDevice ? 'deviceTitle' : 'reportDevice')}
value={multiDevice ? deviceIds : deviceId || ''}
onChange={(e) => dispatch(multiDevice ? devicesActions.selectIds(e.target.value) : devicesActions.selectId(e.target.value))}
multiple={multiDevice}
>
{Object.values(devices).sort((a, b) => a.name.localeCompare(b.name)).map((device) => (
<MenuItem key={device.id} value={device.id}>{device.name}</MenuItem>
))}
</Select>
</FormControl>
<SelectField
label={t(multiDevice ? 'deviceTitle' : 'reportDevice')}
data={Object.values(devices).sort((a, b) => a.name.localeCompare(b.name))}
value={multiDevice ? deviceIds : deviceId}
onChange={(e) => dispatch(multiDevice ? devicesActions.selectIds(e.target.value) : devicesActions.selectId(e.target.value))}
multiple={multiDevice}
fullWidth
/>
</div>
)}
{includeGroups && (
<div className={classes.filterItem}>
<FormControl fullWidth>
<InputLabel>{t('settingsGroups')}</InputLabel>
<Select
label={t('settingsGroups')}
value={groupIds}
onChange={(e) => dispatch(reportsActions.updateGroupIds(e.target.value))}
multiple
>
{Object.values(groups).sort((a, b) => a.name.localeCompare(b.name)).map((group) => (
<MenuItem key={group.id} value={group.id}>{group.name}</MenuItem>
))}
</Select>
</FormControl>
<SelectField
label={t('settingsGroups')}
data={Object.values(groups).sort((a, b) => a.name.localeCompare(b.name))}
value={groupIds}
onChange={(e) => dispatch(reportsActions.updateGroupIds(e.target.value))}
multiple
fullWidth
/>
</div>
)}
{button !== 'schedule' ? (
Expand Down Expand Up @@ -175,7 +165,7 @@ const ReportFilter = ({ children, handleSubmit, handleSchedule, showOnly, ignore
</div>
<div className={classes.filterItem}>
<SelectField
value={calendarId || 0}
value={calendarId}
onChange={(event) => setCalendarId(Number(event.target.value))}
endpoint="/api/calendars"
label={t('sharedCalendar')}
Expand Down
2 changes: 1 addition & 1 deletion modern/src/settings/ComputedAttributePage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ const ComputedAttributePage = () => {
</AccordionSummary>
<AccordionDetails className={classes.details}>
<SelectField
value={deviceId || 0}
value={deviceId}
onChange={(e) => setDeviceId(Number(e.target.value))}
endpoint="/api/devices"
label={t('sharedDevice')}
Expand Down
5 changes: 2 additions & 3 deletions modern/src/settings/DevicePage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ const DevicePage = () => {
</AccordionSummary>
<AccordionDetails className={classes.details}>
<SelectField
value={item.groupId || 0}
value={item.groupId}
onChange={(event) => setItem({ ...item, groupId: Number(event.target.value) })}
endpoint="/api/groups"
label={t('groupParent')}
Expand All @@ -125,7 +125,6 @@ const DevicePage = () => {
/>
<SelectField
value={item.category || 'default'}
emptyValue={null}
onChange={(event) => setItem({ ...item, category: event.target.value })}
data={deviceCategories.map((category) => ({
id: category,
Expand All @@ -134,7 +133,7 @@ const DevicePage = () => {
label={t('deviceCategory')}
/>
<SelectField
value={item.calendarId || 0}
value={item.calendarId}
onChange={(event) => setItem({ ...item, calendarId: Number(event.target.value) })}
endpoint="/api/calendars"
label={t('sharedCalendar')}
Expand Down
2 changes: 1 addition & 1 deletion modern/src/settings/GeofencePage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ const GeofencePage = () => {
label={t('sharedDescription')}
/>
<SelectField
value={item.calendarId || 0}
value={item.calendarId}
onChange={(event) => setItem({ ...item, calendarId: Number(event.target.value) })}
endpoint="/api/calendars"
label={t('sharedCalendar')}
Expand Down
2 changes: 1 addition & 1 deletion modern/src/settings/GroupPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ const GroupPage = () => {
</AccordionSummary>
<AccordionDetails className={classes.details}>
<SelectField
value={item.groupId || 0}
value={item.groupId}
onChange={(event) => setItem({ ...item, groupId: Number(event.target.value) })}
endpoint="/api/groups"
label={t('groupParent')}
Expand Down
5 changes: 2 additions & 3 deletions modern/src/settings/NotificationPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ const NotificationPage = () => {
<AccordionDetails className={classes.details}>
<SelectField
value={item.type}
emptyValue={null}
onChange={(e) => setItem({ ...item, type: e.target.value })}
endpoint="/api/notifications/types"
keyGetter={(it) => it.type}
Expand Down Expand Up @@ -102,7 +101,7 @@ const NotificationPage = () => {
/>
{item.notificators?.includes('command') && (
<SelectField
value={item.commandId || 0}
value={item.commandId}
onChange={(event) => setItem({ ...item, commandId: Number(event.target.value) })}
endpoint="/api/commands"
titleGetter={(it) => it.description}
Expand Down Expand Up @@ -138,7 +137,7 @@ const NotificationPage = () => {
</AccordionSummary>
<AccordionDetails className={classes.details}>
<SelectField
value={item.calendarId || 0}
value={item.calendarId}
onChange={(event) => setItem({ ...item, calendarId: Number(event.target.value) })}
endpoint="/api/calendars"
label={t('sharedCalendar')}
Expand Down
4 changes: 1 addition & 3 deletions modern/src/settings/PreferencesPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -264,16 +264,14 @@ const PreferencesPage = () => {
</AccordionSummary>
<AccordionDetails className={classes.details}>
<SelectField
emptyValue={null}
value={attributes.devicePrimary || 'name'}
onChange={(e) => setAttributes({ ...attributes, devicePrimary: e.target.value })}
data={deviceFields}
titleGetter={(it) => t(it.name)}
label={t('devicePrimaryInfo')}
/>
<SelectField
emptyValue=""
value={attributes.deviceSecondary || ''}
value={attributes.deviceSecondary}
onChange={(e) => setAttributes({ ...attributes, deviceSecondary: e.target.value })}
data={deviceFields}
titleGetter={(it) => t(it.name)}
Expand Down
3 changes: 1 addition & 2 deletions modern/src/settings/ServerPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,7 @@ const ServerPage = () => {
</Select>
</FormControl>
<SelectField
value={item.attributes.timezone || ''}
emptyValue=""
value={item.attributes.timezone}
onChange={(e) => setItem({ ...item, attributes: { ...item.attributes, timezone: e.target.value } })}
endpoint="/api/server/timezones"
keyGetter={(it) => it}
Expand Down
3 changes: 1 addition & 2 deletions modern/src/settings/UserPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,7 @@ const UserPage = () => {
</Select>
</FormControl>
<SelectField
value={(item.attributes && item.attributes.timezone) || ''}
emptyValue=""
value={item.attributes && item.attributes.timezone}
onChange={(e) => setItem({ ...item, attributes: { ...item.attributes, timezone: e.target.value } })}
endpoint="/api/server/timezones"
keyGetter={(it) => it}
Expand Down
2 changes: 1 addition & 1 deletion modern/src/settings/components/BaseCommandView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const BaseCommandView = ({ deviceId, item, setItem }) => {
return (
<>
<SelectField
value={item.type || ''}
value={item.type}
onChange={(e) => setItem({ ...item, type: e.target.value, attributes: {} })}
endpoint={deviceId ? `/api/commands/types?${new URLSearchParams({ deviceId }).toString()}` : '/api/commands/types'}
keyGetter={(it) => it.type}
Expand Down