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: disable download button until map is rendered #3072

Merged
merged 20 commits into from
Mar 5, 2024

Conversation

turban
Copy link
Contributor

@turban turban commented Dec 4, 2023

Implements https://dhis2.atlassian.net/browse/DHIS2-16357

This PR will make sure the download map button is disabled if the map is still being rendered.

Related PR: dhis2/maps-gl#552

For push analytics, the map can be rendered when there is no disabled attribute on the Download button.

The scope of this PR was extended slightly by @HendrikThePendric. It now also contains changes that ensure certain class names are in place and the app is served with a JSON instructions file.

ezgif com-video-to-gif (2)

@dhis2-bot
Copy link
Contributor

dhis2-bot commented Dec 4, 2023

🚀 Deployed on https://pr-3072--dhis2-maps.netlify.app

@dhis2-bot dhis2-bot temporarily deployed to netlify December 4, 2023 16:57 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 4, 2023 17:09 Inactive
Copy link

cypress bot commented Dec 4, 2023

2 flaky tests on run #3028 ↗︎

0 59 15 0 Flakiness 2

Details:

Merge dd0781a into fcc5eaa...
Project: maps Commit: aaecd899fe ℹ️
Status: Passed Duration: 03:24 💡
Started: Mar 5, 2024 12:47 PM Ended: Mar 5, 2024 12:50 PM
Flakiness  routes.cy.js • 1 flaky test • e2e-chrome-parallel-2.40

View Output

Test Artifacts
Routes > navigation by url changes > navigates from currentAnalyticalObject to saved map in download mode Screenshots
Flakiness  basemaps.cy.js • 1 flaky test • e2e-chrome-parallel-2.40

View Output

Test Artifacts
Basemap checks > open map with no basemap uses fallback basemap (OSM Light) when system default basemap is not set Screenshots

Review all test suite changes for PR #3072 ↗︎

Copy link
Contributor

@HendrikThePendric HendrikThePendric left a comment

Choose a reason for hiding this comment

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

I'll refrain from commenting on any other part of this PR than the disabled attribute of the button, which is exactly as we agreed, so approved from my side.

<Button primary onClick={onDownload}>
<Button
primary
disabled={!isRendered}
Copy link
Contributor

Choose a reason for hiding this comment

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

Great! In puppeteer I can wait until the button is enabled and then take a screenshot.

@dhis2-bot dhis2-bot temporarily deployed to netlify December 11, 2023 12:10 Inactive
@turban
Copy link
Contributor Author

turban commented Dec 11, 2023

Added an isPushAnalytics param to the url. if this is not set to true, the Download button will not be disabled while the maps is loading/rendering.

@dhis2-bot dhis2-bot temporarily deployed to netlify January 23, 2024 14:15 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify February 5, 2024 10:39 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify February 5, 2024 11:24 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify February 5, 2024 11:42 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify February 15, 2024 08:26 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify March 1, 2024 09:40 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify March 1, 2024 10:18 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify March 5, 2024 12:37 Inactive
@HendrikThePendric HendrikThePendric merged commit 2cadb44 into dev Mar 5, 2024
15 checks passed
@HendrikThePendric HendrikThePendric deleted the feat/map-rendered branch March 5, 2024 12:59
jenniferarnesen added a commit that referenced this pull request Mar 18, 2024
…l map is rendered (#3072)

* feat: disable download button until map is rendered

* chore: code cleaning

* chore: code cleaning

* chore: code comment

* fix: isPushAnalytics url param

* fix: isPushAnalytics url param

* chore: update @dhis2/analytics and deduplicate deps

* chore: read single url param

* fix: ensure isDownload is a bool to avoid prop-types error

* fix: add class-names for push-analytics

* fix: check download param when navigating to new

* fix: prevent enabling download button while loading mask is showing

* fix: add class to map container when no map id is set

* fix: improve hover states and add consistent spacing [UX-161] (#3121)

* fix: make `dhis2-map-new` class independent of downloadMode

* feat: add push analytics instructions

* chore: upgrade @dhis2/maps-gl

---------

Co-authored-by: HendrikThePendric <hendrik@dhis2.org>
Co-authored-by: Jen Jones Arnesen <jennifer@dhis2.org>
Co-authored-by: Joseph John Aas Cooper <joe@dhis2.org>
dhis2-bot added a commit that referenced this pull request Mar 18, 2024
# [100.5.0](v100.4.1...v100.5.0) (2024-03-18)

### Bug Fixes

* navigating by the changing url ignored download mode and interpretation id parameters ([#3125](#3125)) ([fcc5eaa](fcc5eaa))
* **translations:** sync translations from transifex (dev) ([#3132](#3132)) ([7f5d178](7f5d178))
* improve hover states and add consistent spacing [UX-161] ([#3121](#3121)) ([3f9e667](3f9e667))

### Features

* add ability to add GeoJSON URL external layers ([#3127](#3127)) ([fbdf0b0](fbdf0b0)), closes [#3142](#3142) [#3144](#3144) [#3145](#3145) [#3149](#3149) [#3148](#3148) [#3147](#3147) [#3143](#3143) [#3151](#3151) [#3154](#3154)
* when rendering for push analytics, disable download button until map is rendered ([#3072](#3072)) ([4b1076c](4b1076c)), closes [#3121](#3121)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants