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

Adding Markdown support to Legend text #1279

Merged
1 change: 1 addition & 0 deletions frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
"react-draggable": "^4.4.3",
"react-i18next": "^11.15.4",
"react-map-gl": "^7.1.0",
"react-markdown": "^8.0.7",
"react-pdf": "^5.7.2",
"react-range-slider-input": "^3.0.7",
"react-redux": "^7.2.0",
Expand Down
28 changes: 26 additions & 2 deletions frontend/src/components/MapView/Legends/LegendItem/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import { toggleRemoveLayer } from 'components/MapView/LeftPanel/layersPanel/MenuItem/MenuSwitch/SwitchItem/utils';
import { opacitySelector, setOpacity } from 'context/opacityStateSlice';
import { lightGrey } from 'muiTheme';
import Markdown from 'react-markdown';
import LoadingBar from '../LoadingBar';

// Children here is legendText
Expand Down Expand Up @@ -208,10 +209,28 @@
}
return (
<Grid item>
<Typography variant="h5">{children}</Typography>
{typeof children === 'string' ? (
<Markdown
components={{
p: ({ children }: { children: React.ReactNode }) => (

Check warning on line 215 in frontend/src/components/MapView/Legends/LegendItem/index.tsx

View workflow job for this annotation

GitHub Actions / frontend_tests (ubuntu-latest)

'children' is already declared in the upper scope
<Typography
variant="h5"
className={classes.legendTextMarkdown}
>
{children}
</Typography>
),
}}
allowedElements={['p', 'h5', 'strong', 'em', 'a']}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason to limit elements here? If so, it might be useful to reference allowed elements somewhere else. In the readme maybe or next to the the legendText type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Many of the theoretically allowable elements would not have acceptable formatting here (thinking of examples like tables, lists, code, etc). Rather than try to build reasonably acceptable formatting for all of these elements, I decided to limit to the elements I believe this feature request is intended for.

Good idea, I'll add a note to the readme what exactly is supported

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated ✅

>
{children}
</Markdown>
) : (
<Typography variant="h5">{children}</Typography>
)}
</Grid>
);
}, [children]);
}, [children, classes.legendTextMarkdown]);

return (
<ListItem disableGutters dense>
Expand Down Expand Up @@ -321,6 +340,11 @@
width: 28,
lineHeight: '36px',
},
legendTextMarkdown: {
'& a': {
textDecoration: 'underline',
},
},
});

interface LegendItemProps
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/setupTests.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import React from 'react';
// jest-dom adds custom jest matchers for asserting on DOM nodes.
// allows you to do things like:
// expect(element).toHaveTextContent(/react/i)
Expand Down Expand Up @@ -42,6 +43,11 @@ jest.mock('@react-pdf/renderer', () => ({
Font: { register: () => {} },
}));

// https://github.com/remarkjs/react-markdown/issues/635
jest.mock('react-markdown', () => (props: { children: React.ReactNode }) => {
return <>{props.children}</>;
});
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 is unfortunately, I would have rather rendered this properly in our jest tests. However, the alternative approach seemed more frustrating to maintain than the value it would provide.

See: remarkjs/react-markdown#635

Example alternative:
Screenshot 2024-06-18 at 10 00 54 AM


jest.mock('max-inscribed-circle', () => ({}));

function stubMuiComponent(componentName: string) {
Expand Down
Loading
Loading