JIDEA-183: Use page route to determine whether globe component is loaded #90
Conversation
david-mears-2
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #90 +/- ##
==========================================
- Coverage 1.62% 1.39% -0.24%
==========================================
Files 54 54
Lines 146163 146175 +12
Branches 306 310 +4
==========================================
- Hits 2379 2035 -344
- Misses 143768 144123 +355
- Partials 16 17 +1 ☔ View full report in Codecov by Sentry. |
|
layouts/default.vue
Outdated
| if (!loadGlobeComponent.value) { | ||
| // Set the loadGlobeComponent value here, rather than using a computed property, since the app store initializes with the assumption | ||
| // that largeScreen is true, and so using a computed would mean that we always try to load the globe on the initial page load. | ||
| loadGlobeComponent.value = appStore.largeScreen && pagesUsingGlobe.includes(route.name as string); |
There was a problem hiding this comment.
Could you do this here?
| loadGlobeComponent.value = appStore.largeScreen && pagesUsingGlobe.includes(route.name as string); | |
| loadGlobeComponent.value = showGlobe.value; |
largeScreen should be set correctly at the point where you check its value here.
Also, this isn't checking globeParameter as showGlobeis, but feels like that's relevant for loading too..
| watch(() => route.name, () => { | ||
| setGlobeComponent(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
I didn't expect this pick up the case where you start off with a small window (but on a large screen) which you then expand, so globe should load. But it did! 😕 Does it do a reload when the window size changes?
There was a problem hiding this comment.
Well, if I put in a console.log into this watch, it only fires when I change the route, and correctly not when I change the screen size. What is the confusion?
There was a problem hiding this comment.
If you start off with a small window/screen, you don't want it to load the globe initially... but then you do want it to load if you expand the window size. Which it does! But I'm not sure how it's doing that when it's not watching for changes to the screen size which would setGlobeComponent to set the loadGlobeComponent value.
There was a problem hiding this comment.
onMounted has window.addEventListener("resize", setScreenSize);
setScreenSize runs setGlobeComponent();
There was a problem hiding this comment.
and setGlobeComponent sets loadGlobeComponent.value
There was a problem hiding this comment.
Sorry, yes, I managed to completely fail to see that line! 👓
david-mears-2
deleted the
jidea-183-dont-load-globe-component-on-about-page
branch
Context: https://mrc-ide.myjetbrains.com/youtrack/issue/JIDEA-183/Make-chart-work-with-v-if
In this PR we (continue to) use Nuxt's 'dynamic imports' feature (hence
LazyGlobe, which loads lazily) to determine when to load the Globe component. Till now, it was loading on the About page (as long as largeScreen was true) unnecessarily.