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

Bugfix for loss of map graph display when in mobile #1381

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

tien-han
Copy link
Contributor

@tien-han tien-han commented Nov 6, 2024

Description

This bug fix removes the styling alignItems: 'center' from the div wrapper around the PlotNavComponent, which displays the graphs in the application. With this change, the entire map should be available via scrolling.

Explanation for bug:
The styling did not affect other maps as they were drawn using Plot while considering the viewport's width and height in terms of percentages (100% of width and height). The styling caused this bug for map graphs because an image is used (which is not adjusted to the viewport's height and width), and the styling caused the "start" of the map to be at the center.

Fixes #1347

Type of change

N/A

  • Note merging this changes the database configuration.
  • This change requires a documentation update

Checklist

  • I have followed the OED pull request ideas
  • [N/A] I have removed text in ( ) from the issue request
  • You acknowledge that every person contributing to this work has signed the OED Contributing License Agreement and each author is listed in the Description section.

Limitations

No limitations

Manual Tests Done

I manually tested the app's behavior pre and post fix for both desktop and mobile viewport, considering:

  1. Line graphs still display properly.
  2. Bar graphs still display properly.
  3. Compare graphs still display properly.
  4. Radar graphs still display properly.
  5. Map graphs now display properly.
  6. Navigating between graphs and the Menu appears to be working properly.

Note: Happy to provide screenshots of all other maps still working properly, but will only upload the working map view to start with unless more is requested.

post-fix-map-mobile.mov

@tien-han
Copy link
Contributor Author

tien-han commented Nov 6, 2024

Note: I have manually tested that this change will fix the issue and will not break any other existing feature but will move this PR's status from draft -> review when my teammates can confirm the same on their local development environments.

@tien-han tien-han marked this pull request as ready for review November 7, 2024 00:04
@cmatthews444
Copy link
Contributor

1347.mp4

@SageMar
Copy link
Contributor

SageMar commented Nov 7, 2024

Here is a video of it working on my Windows machine in Firefox browser as well:

Issue1381.mp4

Copy link
Member

@huss huss left a comment

Choose a reason for hiding this comment

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

Thanks to @tien-han, @cmatthews444 & @SageMar for another contribution to OED. This is a simple but effective fix for the issue. Review found no issues and I also tested by using the web browser Responsive Design Mode where all worked well. I've never had a team supply video of the running changes and they also demonstrates that all is working well. Congratulations on another merged PR.

@huss huss merged commit be1abbf into OpenEnergyDashboard:development Nov 7, 2024
3 checks passed
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.

lose parts of map on narrow screen
4 participants