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: Map offset on edge #728

Merged
merged 2 commits into from
Sep 20, 2022
Merged

fix: Map offset on edge #728

merged 2 commits into from
Sep 20, 2022

Conversation

ptbrowne
Copy link
Collaborator

@ptbrowne ptbrowne commented Sep 20, 2022

We have logic to watch the height of the container for chart and table
in published view. This is to ensure that when switching from table
to chart, we have no jump of content.
Before, we would watch the height automatically and it sometimes would
make the container a bit too short to accomodate the map when it mounts.
I suspect it would for a brief time show scrollbars on Edge, and those
scrollbars would offset the layers that were drawn at this point.

Now, we measure the height just before switching to the other view,
instead of doing it continuously. This ensures that the height is stable
and it removes the geojson layers offset bug on Edge.

fix #723

@vercel
Copy link

vercel bot commented Sep 20, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
visualization-tool ✅ Ready (Inspect) Visit Preview Sep 20, 2022 at 10:08AM (UTC)

Copy link
Collaborator

@bprusinowski bprusinowski left a comment

Choose a reason for hiding this comment

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

LGTM, really nice! 👏

Nit: I wonder if it makes sense to maybe extract this logic into a hook, as I think it's duplicated right now across chart preview and chart published? What do you think?

app/components/chart-preview.tsx Outdated Show resolved Hide resolved
@ptbrowne
Copy link
Collaborator Author

Nit: I wonder if it makes sense to maybe extract this logic into a hook, as I think it's duplicated right now across chart preview and chart published? What do you think?

Done 👍

@ptbrowne ptbrowne merged commit e6d090b into main Sep 20, 2022
@ptbrowne ptbrowne deleted the fix/map-offset-edge branch September 20, 2022 10:09
@Rdataflow
Copy link
Contributor

@ptbrowne thx for your fix.

unfortunately the fix only helped for published maps. the error ist still present in edit mode :-(

cc @sosiology

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.

map: geographical offset
3 participants