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

LF-4627 [Nice to have] Implement Add to Map redesign #3715

Merged
merged 20 commits into from
Mar 28, 2025
Merged

LF-4627 [Nice to have] Implement Add to Map redesign #3715

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
b95ff09
LF-4627 Render closed drawer inert (cannot be tabbed to)
kathyavini Mar 11, 2025
dad66be
Add left direction option to SIDE_DRAWER variant
kathyavini Mar 13, 2025
05a04f2
Update Stories. Also add story for addBackdrop: false
kathyavini Mar 13, 2025
cab9ede
Remove unnecessary prop (DesktopDrawerVariant has no relevance in mob…
kathyavini Mar 13, 2025
438e513
Make MapDrawer into a left-opening SIDE_DRAWER
kathyavini Mar 13, 2025
431258e
Create and use new styling class to pass to sideDrawer without impact…
kathyavini Mar 13, 2025
33f48be
Update 'add' icon and text
kathyavini Mar 14, 2025
698b9ed
Add a hover state to the map menu items
kathyavini Mar 14, 2025
f653e66
Update the section (area, lines, points) headers
kathyavini Mar 14, 2025
49f7f66
Merge branch 'integration' into LF-4627-nice-to-have-implement-add-to…
kathyavini Mar 14, 2025
bb1593d
LF-4627 Close Add to Map drawer when unmounting the Map component
kathyavini Mar 14, 2025
5558b0b
LF-4627 Fix console error from incorrect capitalization on propTypes
kathyavini Mar 14, 2025
6d1166a
LF-4627 Small adjusts -- comments
kathyavini Mar 14, 2025
ba2de86
LF-4627 Adjust Drawer CSS classes to allow for visual properties to b…
kathyavini Mar 18, 2025
da2ac46
LF-4627 Create option for a drawer with shadow visible on scroll; app…
kathyavini Mar 19, 2025
9d8c503
Revert "LF-4627 Create option for a drawer with shadow visible on scr…
kathyavini Mar 19, 2025
9475217
LF-4627 Give all drawers a border radius of 4px
kathyavini Mar 19, 2025
f59ab3b
LF-4627 Add 'desktop' to sideDrawer classes and props
kathyavini Mar 24, 2025
863e453
Merge branch 'integration' into LF-4627-nice-to-have-implement-add-to…
kathyavini Mar 26, 2025
96017ce
LF-4627 Create union type
kathyavini Mar 26, 2025
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
19 changes: 15 additions & 4 deletions packages/webapp/src/components/Drawer/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@ interface DrawerProps {
buttonGroup?: React.ReactNode;
fullHeight?: boolean;
desktopVariant?: DesktopDrawerVariants;
sideDrawerDirection?: 'left' | 'right';
Copy link
Collaborator

Choose a reason for hiding this comment

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

This only affects the styles applied in desktop, right? Should this be desktopSideDrawerDirection instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@antsgar Correct, because mobile has no variants! I'm definitely on board with rename but I just want to make sure it's not suggesting there is something like a mobileSideDrawer -- do you think that's clear enough from the props if this change is made?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe sideDrawerDesktopVariantDirection would be clearer? Although perhaps too long 🤔

It might also be good to make the typing more explicit so that setting the direction to left or right is only applicable if the right desktop variant is selected. Something like this could work

type CommonDrawerProps = {
  title: NonNullable<string | React.ReactNode>;
  isOpen: boolean;
  onClose: () => void;
  children: React.ReactNode;
  buttonGroup?: React.ReactNode;
  fullHeight?: boolean;
  isCompactSideMenu?: boolean; // only needed for left side drawer placement
  addBackdrop?: boolean;
  classes?: {
    modal?: string;
    drawerBackdrop?: string;
    drawerHeader?: string;
    drawerContent?: string;
    drawerContainer?: string; // applied to all drawers
    sideDrawerContainer?: string; // side drawer only
  };
}

type DrawerProps = CommonDrawerProps & ({
  desktopVariant?: DesktopDrawerVariants.DRAWER | DesktopDrawerVariants.MODAL;
  sideDrawerDirection: never;
} | {
  desktopVariant: DesktopDrawerVariants.SIDE_DRAWER;
  sideDrawerDirection: 'left' | 'right';
});

and then a TS error would show up if you try to set up the direction but forgot to set up the right variant.

Copy link
Collaborator Author

@kathyavini kathyavini Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Ah I really did want to split up types (for isCompactSideMenu actually) but I removed it because I thought the union type was so much harder to read and the import was in a JS file anyway. This union syntax I like more though!

But the code above isn't working for me like this (even in .tsx files):

and then a TS error would show up if you try to set up the direction but forgot to set up the right variant.

Did it work for you? Instead I only get an error in the valid case of passing neither desktopVariant nor sideDrawer, and I haven't been able to resolve this... 😓 (or actually that one can be resolved by sideDrawerDirection?: never; but then for sure I don't think it would be possible to flag supplying a direction but not a variant)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it work like this?

type DrawerProps = CommonDrawerProps & ({
  desktopVariant?: DesktopDrawerVariants.DRAWER | DesktopDrawerVariants.MODAL;
  sideDrawerDirection?: never;
} | {
  desktopVariant: DesktopDrawerVariants.SIDE_DRAWER;
  sideDrawerDirection?: 'left' | 'right';
})

I'm getting an error if I try to pass a direction without a variant, and no error if I pass neither.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So strange! I don't get an error if I omit a variant -- only if I intentionally set the wrong one. I thought the default setting to DesktopDrawerVariants.MODAL had something to do with it? 🤔 This could also be fixed by requiring a variant, too, but most of the app uses the default (and is in JSX anyway)

Screenshot 2025-03-26 at 11 01 50 AM

In any case I still like the union types as a form of documentation, so I'll put them in!

isCompactSideMenu?: boolean; // only needed for left side drawer placement
addBackdrop?: boolean;
classes?: {
modal?: string;
drawerBackdrop?: string;
drawerHeader?: string;
drawerContent?: string;
drawerContainer?: string;
drawerContainer?: string; // applied to all drawers
sideDrawerContainer?: string; // side drawer only
};
}

Expand All @@ -58,11 +61,15 @@ const Drawer = ({
},
fullHeight,
desktopVariant = DesktopDrawerVariants.MODAL,
sideDrawerDirection = 'right',
isCompactSideMenu,
addBackdrop = true,
}: DrawerProps) => {
const theme = useTheme();
const isDesktop = useMediaQuery(theme.breakpoints.up('sm'));

const isSideDrawer = isDesktop && desktopVariant === DesktopDrawerVariants.SIDE_DRAWER;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, maybe it'd be more explicit as isDesktopSideDrawer?

Copy link
Collaborator Author

@kathyavini kathyavini Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I guess my original question was a bit unclear here -- I was worried if the need to add desktop to the sideDrawer props and booleans means something is fundamentally unclear that variants (DesktopDrawerVariants) cannot apply to mobile (and if so, I would worry this also needs fixing). But maybe the prop name is fix enough? I'll add those first regardless of whether the types can be fixed up!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure, perhaps in the future every desktop variant can have its own props and we can pass them in an object that's called something like desktopVariantProps or such? I think that'd be clearer and more flexible, but probably overkill at this point

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a really nice idea actually! I'll go for the union types for now since they are most of what we need, but I think that would have been better.


return isDesktop && desktopVariant === DesktopDrawerVariants.MODAL && isOpen ? (
<ModalComponent
className={classes.modal}
Expand All @@ -88,13 +95,17 @@ const Drawer = ({
<div
className={clsx(
styles.drawer,
isDesktop && desktopVariant === DesktopDrawerVariants.SIDE_DRAWER
? styles.sideDrawer
: styles.bottomDrawer,
isSideDrawer ? styles.sideDrawer : styles.bottomDrawer,
isSideDrawer && styles[sideDrawerDirection],
sideDrawerDirection === 'left' && isCompactSideMenu
? styles.withCompactSideMenu
: styles.withExpandedSideMenu,
fullHeight && styles.fullHeight,
isOpen ? styles.openD : '',
classes.drawerContainer,
isSideDrawer && classes.sideDrawerContainer,
)}
inert={!isOpen ? '' : null}
>
<div className={clsx(styles.header, classes.drawerHeader)}>
<div className={styles.title}>{title}</div>
Expand Down
26 changes: 24 additions & 2 deletions packages/webapp/src/components/Drawer/style.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,37 @@
.sideDrawer {
top: var(--global-navbar-height);
max-height: calc(100vh - var(--global-navbar-height));
right: -100%;
}

.sideDrawer.right {
border-bottom-left-radius: 10px;
width: 416px;
right: -100%;

&.openD {
right: 0;
}
}

.sideDrawer.left {
border-radius: 0 10px 10px 0;
}

.sideDrawer.left.withExpandedSideMenu {
left: -100%;

&.openD {
left: var(--global-side-menu-width);
}
}

.sideDrawer.left.withCompactSideMenu {
left: -100%;

&.openD {
left: var(--global-compact-side-menu-width);
}
}

.header {
display: flex;
justify-content: space-between;
Expand Down
1 change: 0 additions & 1 deletion packages/webapp/src/components/Form/ContextForm/WithPageTitle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export const WithPageTitle = ({
<ClickAwayListener onClickAway={onClickAway} mouseEvent="onMouseDown" touchEvent="onTouchStart">
<div>
<Layout>
{/* @ts-expect-error */}
<MultiStepPageTitle
title={title}
onGoBack={onGoBack}
Expand Down
2 changes: 2 additions & 0 deletions packages/webapp/src/components/Map/Footer/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export default function PureMapFooter({
filterSettings={filterSettings}
availableFilterSettings={availableFilterSettings}
onMenuItemClick={onFilterMenuClick}
isCompactSideMenu={isCompactSideMenu}
/>
<MapDrawer
key={'add'}
Expand All @@ -104,6 +105,7 @@ export default function PureMapFooter({
drawerDefaultHeight={window.innerHeight - 156}
headerTitle={t('FARM_MAP.MAP_FILTER.ADD_TITLE')}
onMenuItemClick={onAddMenuClick}
isCompactSideMenu={isCompactSideMenu}
/>
</TourProviderWrapper>
);
Expand Down
13 changes: 11 additions & 2 deletions packages/webapp/src/components/MapDrawer/MapDrawerMenuItem.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import clsx from 'clsx';
import { useTranslation } from 'react-i18next';
import ListItem from '@mui/material/ListItem';
import ListItemIcon from '@mui/material/ListItemIcon';
import ListItemText from '@mui/material/ListItemText';
Expand All @@ -6,18 +8,22 @@ import { colors, DefaultThemeProvider } from '../../assets/theme';
import PropTypes from 'prop-types';
import { Main } from '../Typography';
import styles from './styles.module.scss';
import Icon from '../Icons';

export default function MapDrawerMenuItem({
onClick,
isFilterMenuItem,
isFiltered,
children,
name,
}) {
const { t } = useTranslation();

return (
<DefaultThemeProvider>
<ListItem
button
style={{ backgroundColor: isFiltered ? '#F3F6FB' : 'white' }}
className={clsx(isFiltered ? styles.filteredItem : styles.menuItem)}
onClick={onClick}
>
<ListItemIcon>{children}</ListItemIcon>
Expand All @@ -29,7 +35,10 @@ export default function MapDrawerMenuItem({
{isFilterMenuItem ? (
<EyeToggleIcon isFiltered={isFiltered} />
) : (
<span className={styles.plusIcon}>+</span>
<div className={styles.plusAdd}>
<Icon iconName={'PLUS_CIRCLE'} className={styles.plusIcon} />
{t('common:ADD')}
</div>
)}
</ListItem>
</DefaultThemeProvider>
Expand Down
25 changes: 16 additions & 9 deletions packages/webapp/src/components/MapDrawer/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { useTranslation } from 'react-i18next';
import PropTypes from 'prop-types';
import { locationEnum } from '../../containers/Map/constants';
import MapDrawerMenuItem from './MapDrawerMenuItem';
import Drawer from '../Drawer';
import Drawer, { DesktopDrawerVariants } from '../Drawer';
import styles from './styles.module.scss';

export default function MapDrawer({
Expand All @@ -33,6 +33,7 @@ export default function MapDrawer({
filterSettings,
availableFilterSettings,
headerTitle,
isCompactSideMenu,
}) {
const { t } = useTranslation();

Expand Down Expand Up @@ -200,9 +201,8 @@ export default function MapDrawer({
)}

{!!areaImgDict.length && (
<Label className={styles.label}>
<Label className={clsx(styles.label, !!filterSettings && styles.sectionLabelPadding)}>
{t('FARM_MAP.MAP_FILTER.AREAS')}
<span className={styles.labelDivider} />
</Label>
)}
{areaImgDict.map(({ key, name, icon }) => {
Expand All @@ -220,10 +220,11 @@ export default function MapDrawer({
})}

{!!lineImgDict.length && (
<Label className={styles.label}>
{t('FARM_MAP.MAP_FILTER.LINES')}
<span className={styles.labelDivider} />
</Label>
<>
<Label className={clsx(styles.label, styles.sectionLabelPadding)}>
{t('FARM_MAP.MAP_FILTER.LINES')}
</Label>
</>
)}
{lineImgDict.map(({ key, name, icon }) => (
<MapDrawerMenuItem
Expand All @@ -238,9 +239,8 @@ export default function MapDrawer({
))}

{!!pointImgDict.length && (
<Label className={styles.label}>
<Label className={clsx(styles.label, styles.sectionLabelPadding)}>
{t('FARM_MAP.MAP_FILTER.POINTS')}
<span className={styles.labelDivider} />
</Label>
)}
{pointImgDict.map(({ key, name, icon }) => (
Expand All @@ -264,7 +264,14 @@ export default function MapDrawer({
data-cy="map-drawer"
title={headerTitle}
isOpen={showMapDrawer}
desktopVariant={DesktopDrawerVariants.SIDE_DRAWER}
sideDrawerDirection="left"
isCompactSideMenu={isCompactSideMenu}
addBackdrop={false}
onClose={() => setShowMapDrawer(false)}
classes={{
sideDrawerContainer: styles.sideDrawerContainer,
}}
>
{list()}
</Drawer>
Expand Down
52 changes: 41 additions & 11 deletions packages/webapp/src/components/MapDrawer/styles.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* GNU General Public License for more details, see <https://www.gnu.org/licenses/>.
*/

.headerContentContainer {
.headerContentContainer {
padding-bottom: 8px;
}

Expand All @@ -34,19 +34,49 @@
}

.label {
height: 24px;
color: var(--Colors-Neutral-Neutral-600);
font-size: 18px;
font-weight: 700;
line-height: 24px;
}

.labelDivider {
display: inline-block;
border-top: 1px solid;
width: calc(100% - 80px);
transform: translate(11px, -3px);
color: #C4C4C4;
.sectionLabelPadding {
padding-top: 32px;
}

// MapDrawerMenuItem.jsx
.menuItem {
background-color: white;
&:hover {
background-color: var(--Colors-Primary-Primary-teal-50);
}
}

.filteredItem {
background-color: #f3f6fb;
}

.plusAdd {
display: flex;
padding: 2px;
align-items: center;
gap: 4px;

color: var(--Colors-Primary-Primary-teal-700);
font-weight: 600;
line-height: 24px;
}

.plusIcon {
color: var(--teal700);
font-size: 42px;
line-height: 16px;
background-color: transparent;
}

// Drawer
.sideDrawerContainer {
top: calc(
var(--global-navbar-height) + var(--global-map-header-height) - 1px /* sm gap without adjust */
);
max-height: calc(100vh - var(--global-navbar-height) - var(--global-map-header-height));
min-width: 432px;
border-radius: 0px 4px 4px 0px;
}
3 changes: 1 addition & 2 deletions packages/webapp/src/components/Navigation/SideMenu/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { matchPath } from 'react-router-dom';
import useExpandable from '../../Expandable/useExpandableItem';
import { ReactComponent as Logo } from '../../../assets/images/middle_logo.svg';
import { useGetMenuItems } from '../../../hooks/useGetMenuItems';
import Drawer, { DesktopDrawerVariants } from '../../Drawer';
import Drawer from '../../Drawer';
import { ReactComponent as CollapseMenuIcon } from '../../../assets/images/nav/collapse-menu.svg';
import styles from './styles.module.scss';
import { getLanguageFromLocalStorage } from '../../../util/getLanguageFromLocalStorage';
Expand Down Expand Up @@ -235,7 +235,6 @@ const PureSideMenu = ({
isOpen={isDrawerOpen}
onClose={onDrawerClose}
fullHeight
desktopVariant={DesktopDrawerVariants.DRAWER}
classes={{
drawerContainer: styles.drawerContainer,
drawerHeader: styles.drawerHeader,
Expand Down
2 changes: 1 addition & 1 deletion packages/webapp/src/components/PageTitle/CancelButton.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function CancelButton({ onCancel, showConfirmCancelModal, setShowConfirmC
);
}

CancelButton.PropTypes = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Completely unrelated but this was my typo from #3710 in two files that was causing a console warning:

Screenshot 2025-03-14 at 3 50 19 PM

CancelButton.propTypes = {
onCancel: PropTypes.func,
showConfirmCancelModal: PropTypes.bool,
setShowConfirmCancelModal: PropTypes.func,
Expand Down
2 changes: 1 addition & 1 deletion packages/webapp/src/components/PageTitle/MultiStepPageTitle/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function MultiStepPageTitle({
}

export default MultiStepPageTitle;
MultiStepPageTitle.PropTypes = {
MultiStepPageTitle.propTypes = {
title: PropTypes.string,
onGoBack: PropTypes.func,
onCancel: PropTypes.func,
Expand Down
3 changes: 3 additions & 0 deletions packages/webapp/src/containers/FeedbackSurvey/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ export default function FeedbackSurvey({
addBackdrop={false}
desktopVariant={DesktopDrawerVariants.SIDE_DRAWER}
fullHeight={true}
classes={{
sideDrawerContainer: styles.sideDrawerContainer,
}}
>
{drawerContent}
</Drawer>
Expand Down
4 changes: 4 additions & 0 deletions packages/webapp/src/containers/FeedbackSurvey/styles.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,7 @@
.content {
height: 100%;
}

.sideDrawerContainer {
min-width: 416px;
}
1 change: 1 addition & 0 deletions packages/webapp/src/containers/Map/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ export default function Map({ history, isCompactSideMenu }) {
}
return () => {
dispatch(canShowSuccessHeader(false));
dispatch(setMapAddDrawerHide(farm_id));
Copy link
Collaborator Author

@kathyavini kathyavini Mar 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This hides the Add to Map Drawer when unmounting Map (i.e. when navigating away). There was no previous need to do so since the backdrop of the modal was basically a click away listener that would close if you clicked on, e.g., the side menu.

Actually -- and I think this is a lowkey bug -- you can see on beta that if you start the "add area" action, which hides the backdrop and modal, and then navigate to a different part of app, the modal is re-opened when you return to Map because the 'showModal' redux state was never touched. I think resetting that state when leaving Map makes more sense!

};
}, []);

Expand Down
Loading
Loading