Fix/showcase filter blink#424
Conversation
Deploying labs-browserpod-previews with
|
| Latest commit: |
e4718db
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://eb21a2e9.labs-browserpod-previews.pages.dev |
| Branch Preview URL: | https://fix-showcase-filter-blink.labs-browserpod-previews.pages.dev |
| @@ -0,0 +1,4 @@ | |||
| --- | |||
| // No-op: filter paint guard is handled in Shell.astro <head>. | |||
There was a problem hiding this comment.
This seems completely spurious, probably the results of multiple attempts by the machine. It's introduced in this same PR.
| } | ||
| </style> | ||
| <script is:inline> | ||
| // Hide filter grids before first paint when the URL carries a filter param, |
There was a problem hiding this comment.
I believe this solution will be a source of confusion, things are normally filtered in ProductFilter.svelte, and we apply this clutch here which is completely unrelated.
I can suggest 2 solutions
- [Easier and less elegant]: In ProductFilter initialize all the content as "not visible" and during the onMount handler setup visibility correctly.
- [Harder but nicer]: I think the reason why onMount is needed here is to have all the DOM element ready since the code fiddles with classes. Another option is to create the elements with the right classes already and use svelte reactivity to update their state. For example, all visible content could be listed in a array and the visibility could be controlled via svelte reactivity depending on the id of the content being present or not in the list.
| if (filterPage) { | ||
| try { | ||
| document.documentElement.classList.add("filtering-pending"); | ||
| setTimeout( |
There was a problem hiding this comment.
I do understand the pattern here, but doing anything on timeouts is a bad practice.
We should reset this styling as the end of the code that actually setup the right final state.
There was a problem hiding this comment.
I can actually see below that the style is correctly removed, which makes this operation redundant.
I can guess the machine put it here as a fallback if this snippet apply to some other page, but that does not make sense, we are explicitly filtering on pages that need it.
| // has rendered its first filtered state. $effect fires after DOM updates and | ||
| // does not run during SSR, so this only executes in the browser. | ||
| $effect(() => { | ||
| document.documentElement.classList.remove("filtering-pending"); |
There was a problem hiding this comment.
This should be enough to reset the state. setTimeout is redundant
2 participants
Here's a summary of what was done:
Root cause: The page uses client-side filtering (client:load), so Svelte only applies the URL filter params after the JS bundle hydrates — by which point the browser has already painted the full unfiltered page.
Fix in three layers:
The result: when opening a filtered URL, the user sees nothing until the correct filtered view is ready, then it fades in smoothly.