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

Adding Markdown support to Legend text #1279

Merged
merged 11 commits into from
Jun 26, 2024
Merged

Adding Markdown support to Legend text #1279

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3dc9d1d
Initial Markdown support
gislawill Jun 18, 2024
882cffb
Merge branch 'master' into 1233-feature-request-support-markdown-in-l…
gislawill Jun 18, 2024
a683cf7
Mock in tests
gislawill Jun 18, 2024
52fb77b
Update snapshot
gislawill Jun 18, 2024
c71f9e9
appease eslint
gislawill Jun 18, 2024
959326c
Updating links to open in new tab
gislawill Jun 18, 2024
93cce38
Add content for QA
gislawill Jun 21, 2024
1e838ea
Merge branch 'master' into 1233-feature-request-support-markdown-in-l…
gislawill Jun 26, 2024
075fe7c
Adding links to legend_text in layers.json
gislawill Jun 26, 2024
295cec8
Adding translation files
gislawill Jun 26, 2024
f32a6b6
Updating Readme
gislawill Jun 26, 2024
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
1 change: 1 addition & 0 deletions frontend/package.json
View file
Open in desktop
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
10 changes: 5 additions & 5 deletions frontend/src/components/MapView/Legends/LegendItem/__snapshots__/index.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ exports[`renders as expected 1`] = `
/>
</div>
<div
class="MuiLinearProgress-root makeStyles-root-10 MuiLinearProgress-colorPrimary MuiLinearProgress-indeterminate"
class="MuiLinearProgress-root makeStyles-root-11 MuiLinearProgress-colorPrimary MuiLinearProgress-indeterminate"
role="progressbar"
>
<div
class="MuiLinearProgress-bar makeStyles-hide-12 MuiLinearProgress-barColorPrimary MuiLinearProgress-bar1Indeterminate"
class="MuiLinearProgress-bar makeStyles-hide-13 MuiLinearProgress-barColorPrimary MuiLinearProgress-bar1Indeterminate"
/>
<div
class="MuiLinearProgress-bar makeStyles-hide-12 MuiLinearProgress-bar2Indeterminate MuiLinearProgress-barColorPrimary"
class="MuiLinearProgress-bar makeStyles-hide-13 MuiLinearProgress-bar2Indeterminate MuiLinearProgress-barColorPrimary"
/>
</div>
<div
Expand All @@ -55,7 +55,7 @@ exports[`renders as expected 1`] = `
style="margin: 8px 0px;"
/>
<div
class="MuiBox-root MuiBox-root-13"
class="MuiBox-root MuiBox-root-14"
>
<mock-tooltip
title="Opacity"
Expand Down Expand Up @@ -89,7 +89,7 @@ exports[`renders as expected 1`] = `
open="false"
>
<div
class="MuiBox-root MuiBox-root-14 memo-opacityBox-3"
class="MuiBox-root MuiBox-root-15 memo-opacityBox-3"
>
<mock-typography
classes="[object Object]"
Expand Down
29 changes: 27 additions & 2 deletions frontend/src/components/MapView/Legends/LegendItem/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import AnalysisDownloadButton from 'components/MapView/Legends//AnalysisDownload
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,29 @@ const LegendItem = memo(
}
return (
<Grid item>
<Typography variant="h5">{children}</Typography>
{typeof children === 'string' ? (
<Markdown
linkTarget="_blank"
components={{
p: ({ children: pChildren }: { children: React.ReactNode }) => (
<Typography
variant="h5"
className={classes.legendTextMarkdown}
>
{pChildren}
</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 +341,11 @@ const styles = () =>
width: 28,
lineHeight: '36px',
},
legendTextMarkdown: {
'& a': {
textDecoration: 'underline',
},
},
});

interface LegendItemProps
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/config/mozambique/layers.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
"base_url": "https://api.earthobservation.vam.wfp.org/ows/",
"date_interval": "days",
"opacity": 0.7,
"legend_text": "Local station data from INAM blended with CHIRP rainfall estimates",
"legend_text": "**Local station data** from INAM blended with [CHIRP](https://www.chc.ucsb.edu/data/chirps) *rainfall estimates*",
"legend": "rfh_16_0_300"
},
"precip_blended_dekad_anomaly": {
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/setupTests.tsx
View file
Open in desktop
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