This repository has been archived by the owner on Sep 30, 2024. It is now read-only.
Fix resize observers #100
Merged
Fix resize observers #100
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Combining the two type components into a single component helps in two
ways:
1. This should result in more efficient updates, so that we don't
destroy and recreate the map whenever we change variable types
2. We don't duplicate layout in our code (things like the heading)
The LoopDisplay component makes the decision of whether to display the
1D or 2D histogram based on the variable type it receives, which keeps
the code in the DiagnosticView component a little easier to read; it
just has to pass down the variable type and let the display do with that
as it will.
There's still a bug in the map that's causing it not to display. I think
I need to distinguish the observations for the map from the values for
the distribution. This will be important later when we introduce
brushing on the histograms, because we don't want to filter the data for
the chart that has a brush applied.
The map expects valid GeoJSON as its data so that it can pass that to the projection's fitSize function, so we need to wrap the filtered data in a FeatureCollection object.
I think we can let the chart components be sized by their containers, rather than specifying an aspect ratio. This should make the layouts more responsive.
Based on a CodePen that creates the responsive layout I want for the application that has a minimum size for the rows and allows them to expand to fill available space, and wraps the columns on narrow viewports. https://codepen.io/darthmall/pen/yLjqLKq/389a32bcec1d37a46887b433cdb0f379 The codepen works the way I'd expect, but the layout is not working correctly in the app. The charts are generally taller than I think they ought to be. This may be the result of the resize observers not firing, so I'm going to fix that bug and see if things start behaving.
For the histograms, I've added a div wrapper to the shadow root so that our resize observers will fire. This looks like it's working, but the charts are still too tall. At least they don't grow infinitely.
First, I set `contain: strict` on both the host element of the web component and on the container with the resize observer. This is significantly more elegant than setting `position: relative; overflow: hidden`, etc. Second, I set up the wrapper on the ChartMap with the fixed resize code, because I think that's what was causing the rows to grow oddly. Since one of the charts in the row was misbehaving, the whole row misbehaved. Using a div container for the resize observer and setting `contain: strict` on everything causes the rows to grow and shrink as needed to fill the available space, but no smaller than the row's min height.
30rem was a little large. 20rem feels like a better minimum size.
esheehan-gsl
force-pushed
the
88-resize-observers-not-firing
branch
from
6ed0f44 to
159ea3f
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.