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

feat(web): link data portal from app #7703

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tonypls
Copy link
Collaborator

@tonypls tonypls commented Jan 8, 2025

Issue

We don't have enough visibility of the data portal in the app

Description

This PR adds download button to the zone header which links to the appropriate page of the data portal

Preview

image

TODO

Only show button when zone has data in the data portal
Finalize design and button placement

Copy link
Member

@VIKTORVAV99 VIKTORVAV99 left a comment

Choose a reason for hiding this comment

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

We need the fix the button in other languages before it can be merged.

Comment on lines +8 to +17
function getCountryToDownload(zoneId?: string) {
if (!zoneId) {
return '';
}

const zoneName = getZoneName(zoneId);
const zoneCountryName =
zoneId && zoneId.includes('-') && getCountryName(zoneId)?.toLowerCase();
return zoneCountryName || zoneName;
}
Copy link
Member

Choose a reason for hiding this comment

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

This don't really work as it gets the translated name of the country. So it breaks for everything except English.

Comment on lines +26 to +32
<Button
href={url}
icon={<Download size={DEFAULT_ICON_SIZE} />}
type="tertiary"
size="md"
aria-label="Download CSV Data"
/>
Copy link
Member

Choose a reason for hiding this comment

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

Can we mark that this is an external link just like we do in the header the desktop view?

);
}

export default DownloadCsv;
Copy link
Member

Choose a reason for hiding this comment

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

Let's Memo it just in case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants