Conversation
👷 Deploy request for pauseai pending review.Visit the deploys page to approve it
|
src/routes/+layout.svelte
Outdated
| // Show the hero on the homepage, but nowhere else | ||
| $: hero = deLocalizeHref($page.url.pathname) === '/' | ||
|
|
||
| $: if (browser && deLocalizeHref($page.url.pathname) === '/india-summit-2026') { |
There was a problem hiding this comment.
No longer needed: CampaignBanner already auto-dismisses when the user navigates to its href page (via the reactive $: block in the component).
| {:else if false} | ||
| <Banner contrast={hero} target="/littlehelpers"> | ||
| {:else if isDateRangeActive({ endsOn: '2024-12-31' })} | ||
| <Banner contrast={hero} id="holiday-littlehelpers" target="/littlehelpers"> |
There was a problem hiding this comment.
Previously had no id, so dismissals didn't persist across page loads. Added one so the close button actually remembers the user's choice.
|
This probably has the same flickering issue #663 fixed but for all users right? I think I didn't really like the manual inclusion in the HTML and procrastinated reviewing it. Maybe we can get Vite to properly bundle a script and link it in the document head? SvelteKit doesn't seem to have a way to add blocking scripts from my research |
|
I moved the blocking logic to a separate file and merged the PR. Can you merge it in here and build on that @RisingOrange ? |
Banners dismissed via localStorage previously stayed hidden forever, and old campaigns were manually disabled with `&& false`. Now banners accept expiresAt/startsAt date props to auto-expire, and dismissals are scoped per campaign id. Extracts shared localStorage logic into bannerStorage.ts, removes debug console.logs and hacky DOM click handler, and fixes Banner target path comparison to use deLocalizeHref.
Campaign banners dismissed via localStorage stayed hidden forever, and
old campaigns were manually disabled with `&& false` — error-prone and
cluttered.
Add isDateRangeActive() helper that parses date-only strings as local
calendar days, and use it in layout conditions to replace `&& false`
gates. This keeps the active/inactive decision in the layout where the
if/else branching happens, avoiding a bug where expired component-level
props would still consume the branch and block fallback banners.
Other improvements:
- Extract safe localStorage helpers (getItem/setItem) into localStorage.ts
- Remove debug console.logs and hacky DOM click handler from Banner
- Add eager path check at init to prevent flash without {#if} wrappers
- Fix Banner target path comparison to use deLocalizeHref
|
Oops sorry didn't see your reaction. Hope I didn't cause a conflict on your end |
|
@Wituareard No problem! I'm still thinking about how best to solve the flash issue. Are you doing scheduled rebuilds (e.g. via GitHub Actions cron + Netlify build hook) for the site? It would help with banner flashes due to the server-side build having evaluated the date conditions on a previous day. It could also help keep prerendered data (like some of the Airtable endpoints) fresh. The GHA could also be used to trigger rebuilds, instead of commits used purely to trigger rebuilds (like Trigger Netlify build to test missing API key behavior). A single workflow could handle both scheduled and manual triggers: # .github/workflows/rebuild.yml
name: Rebuild site
on:
schedule:
- cron: '0 */6 * * *' # every 6 hours
workflow_dispatch:
jobs:
rebuild:
runs-on: ubuntu-latest
steps:
- run: curl -X POST "${{ secrets.NETLIFY_BUILD_HOOK }}"Just needs a Netlify build hook URL added as a repo secret. |
RisingOrange
force-pushed
the
banner-expiration
branch
from
df237fe to
397bb90
Compare
|
Maybe banners could add a svelte:head element to register themselves and we somehow make sure the hiding script is placed after them. Are you suggesting to entirely move the banner expiration to the server? Could work since we probably don't need more accuracy than 24h. Daily builds could be a good idea regardless, didn't know of that feature. |
2 participants
Closes #676
Campaign banners had to be manually disabled after they expired, which was easy to forget. This PR introduces the pattern of using
isDateRangeActive()date checks so banners auto-expire without code changes. Banners also now self-hide on their target page without needing{#if}wrappers in the layout. Also cleans up debug leftovers inBannerand extracts shared localStorage helpers.Changes
&& falsegates on expired banners withisDateRangeActive()checks in layout conditionsisDateRangeActive()helper (src/lib/dateRange.ts) that parses date-only strings as local calendar days (avoids UTC midnight issues withnew Date('YYYY-MM-DD'))getItem/setItemlocalStorage helpers intosrc/lib/localStorage.ts(browser guard + try-catch)Bannercomponent: remove leftoverconsole.logs and redundant DOM click handler, fixtargetpath comparison to usedeLocalizeHrefBannerandCampaignBanner. This hides the banner on its target page during SSR (no flash), removing the need for{#if pathname !== href}wrappers in the layoutDesign decision
I initially added
expiresAt/startsAtprops directly on theBannercomponents, but discovered this doesn't work with the layout's if/else chain: an expired banner still consumes its branch, blocking fallback banners (e.g. GB users would get neither the expired protest banner norNearbyEvent). Moving the date check to the layout condition fixes this since the branch itself evaluates to false.Adding a new campaign banner
{#if isDateRangeActive({ endsOn: '2026-04-15' })} <CampaignBanner href="/my-event" id="my-event"> ... </CampaignBanner> {/if}endsOnis inclusive (end of day, local time).startsOnis also supported for future-dated campaigns.Open question
When a user visits the campaign page directly (e.g.
/brussels-ep-protest-2026),CampaignBannerpermanently hides itself (even on future page loads).Banneronly hides while the user is on that page and comes back on other pages. This inconsistency is pre-existing. Should we align them?Test plan
endsOndate in the future (on a banner in+layout.svelte), confirm banner appearsstartsOnin the future, confirm banner doesn't appear