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

fix: use parent container for fullscreen map (DHIS2-8524) #77

Merged
merged 2 commits into from
Mar 25, 2020

Conversation

turban
Copy link
Collaborator

@turban turban commented Mar 25, 2020

Fixes: https://jira.dhis2.org/browse/DHIS2-8524

Related PR for Maps app: dhis2/maps-app#522

The default behaviour of the fullscreen control is to open the map container in fullscreen. As we also want to include other elements like map title, it's better to use the 2x parent component. This is the same we already use for plugin/dashboard maps, so it simplifies the code.

Screenshot 2020-03-25 at 12 42 28

Split view maps (which consists of multiple maps still need to move one level up in the DOM hierarchy).

Screenshot 2020-03-25 at 12 42 07

The PR also includes a style fix for split view map controls which had a transparent hover style which should be solid.

Before:
Screenshot 2020-03-25 at 12 44 45

After:
Screenshot 2020-03-25 at 12 48 46

Copy link

@martinkrulltott martinkrulltott left a comment

Choose a reason for hiding this comment

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

LGTM, haven't tested locally though but screenshots look reliable enough.

@turban turban merged commit e1290d6 into master Mar 25, 2020
@turban turban deleted the fix/DHIS2-8524 branch March 25, 2020 12:54
dhis2-bot added a commit that referenced this pull request Mar 25, 2020
## [1.0.17](v1.0.16...v1.0.17) (2020-03-25)

### Bug Fixes

* use parent container for fullscreen map (DHIS2-8524) ([#77](#77)) ([e1290d6](e1290d6))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 1.0.17 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants