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

WAITP-1223: Tupaia web map legend #4724

Merged
merged 7 commits into from
Jul 12, 2023
Merged

Conversation

tcaiger
Copy link
Contributor

@tcaiger tcaiger commented Jul 11, 2023

Issue #: WAITP-1223: Tupaia web map legend

Changes:

  • Desktop map legend
  • Mobile map legend
  • Setup hidden values to handle clicking on the map legend

Screenshots:

image

image

image

image

image

image

Copy link
Contributor

@alexd-bes alexd-bes left a comment

Choose a reason for hiding this comment

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

This is looking really good. I have a couple of questions about possible ways to do some things, but they are optional to carry out.

return (
<>
<MobileMapLegend />
<DesktopMapLegend />
<MobileMapLegend Legend={LegendComponent} />
Copy link
Contributor

Choose a reason for hiding this comment

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

This is very minor, but what do you think about having the LegendComponent as children of the different legend wrappers?

</Wrapper>
);
export const DesktopMapLegend = ({ Legend }) => {
return <Wrapper>{Legend}</Wrapper>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if, since this is so simple now, if it can just be a wrapper styled component inside MapLegend?


const SeriesDivider = styled.div`
height: 0;
border-bottom: 1px solid #ffffff22;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want this colour to come from the theme at all? I dunno if it will change ever, but if the theme is a light theme then the divider wouldn't show, for example.

@tcaiger tcaiger merged commit c7c874a into epic-frontend-rewrite Jul 12, 2023
@tcaiger tcaiger deleted the waitp-1223-map-legend branch July 12, 2023 05:38
tcaiger added a commit that referenced this pull request Jul 12, 2023
* setup map legend

* update legend props

* mobile map legend

* Update MobileMapLegend.tsx

* refactor map legend

* fix console warnings
tcaiger added a commit that referenced this pull request Jul 12, 2023
* setup map legend

* update legend props

* mobile map legend

* Update MobileMapLegend.tsx

* refactor map legend

* fix console warnings
tcaiger added a commit that referenced this pull request Jul 13, 2023
* setup map legend

* update legend props

* mobile map legend

* Update MobileMapLegend.tsx

* refactor map legend

* fix console warnings

fix export styles (#4727)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants