Repository navigation
feat(feed): widget rail foundation (data, layout, registry) - #6963
Conversation
The model behind the feed widget rail, not wired into any page yet. The UI and the feed-page wiring follow in a stacked PR. - types: kinds (mirroring daily-api's FeedWidgetKind), Small/Tall/Large sizes on a two-row grid, per-kind config. - layout: first-fit packing with the ad pinned to the third column. A Large counts as two, so after it the ad is the second item. The ad closes up rather than leave a column before it empty, and a lone Small's free cell is offered as a slot. Covered by layout.spec. - registry + components: every kind the API knows, and which ones are built. A stored kind this build lacks is dropped rather than crashing. - useFeedWidgets: read and write Settings.flags.feedWidgets as one setSettings each (updateFlag snapshots and loses concurrent writes), defaulting to the top story alone; undo recomputes against the latest list. - Top story (one lead story, Small/Tall/Large) and Happening Now widgets, sharing the hero's one query through useFeedHeroData, which FeedHero now uses too. - useFeed/Feed: excludedPostIds seeds the dedupe set, so a post the rail shows is kept out of the grid. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
rebelchris
left a comment
There was a problem hiding this comment.
Reviewed the model layer (layout packing, registry, useFeedWidgets, the hero data extraction and excludedPostIds) against daily-api#4446.
I checked the contract against #4446. Kinds, sizes, MAX_FEED_WIDGETS = 12, unique ids, the per-kind config schemas and null as reset all match. One mismatch: the client drops v (inline comment on layout.ts).
Merge/deploy order: #4446 has to be deployed before #6955 ships any write. Every settings write sends the whole flags object, and feedWidgets isn't in clientOnlySettingsFlags, so an API without the field fails GraphQL validation and takes the reader's other settings writes down with it. This PR on its own writes nothing, so it can merge first.
Findings: 1 blocking (writing back the render-filtered list deletes widgets from other clients and strips v), 3 non-blocking (duplicated hero handlers, Small top story has no impression, excluded posts skipped after the ad slot is claimed).
Ran locally on the head commit: the feedWidgets and feeds/hero specs pass (5 suites, 44 tests). I also compiled the new Tailwind classes (via-raw-pepper-90/60, text-white/70, …) against the config, and all of them resolve. I traced the layout packer's ad close-up and gap paths by hand and found no issues beyond the specs. The UI in #6955 isn't covered by this review.
Reviewed by AI.
Review follow-ups on the widget rail foundation. Blocking: useFeedWidgets built every write on the list it rendered, which drops the kinds this build has no component for, rewrites sizes it does not support and keeps only id/kind/size/config. So an edit on an older extension deleted a widget a newer webapp had added, and the first reorder erased daily-api's config version `v`. The hook now keeps two lists: the stored one, checked for shape only and written back intact, and a visible copy for rendering. Reorders merge back into the stored list so the widgets this build skips keep their exact slots, and FeedWidget gains `v`. Also: - FeedHero uses useFeedHeroCardProps instead of its own copy of the handlers, so the hero and the top story widget cannot drift. - The Small top story logs an impression on the carousel's terms (half in view, once per post); clicks on it had nothing to divide by. - An excluded post bails out before getAd, like a pinned one, rather than being skipped as a duplicate after an ad claimed its slot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…to feat/feed-widgets-foundation
daily-api#4446 validates widgets by shape only, so a new kind no longer lands in the API first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rebelchris
left a comment
There was a problem hiding this comment.
Re-review of 4d44279 and d6749f3 (head f74586e), against the earlier review.
- Write-back blocker: mostly fixed. Writes now build on the stored list. Unknown kinds keep their slots, and
vand other fields survive. One piece is left: a reorder still saves the size substituted for display (inline onlayout.ts). FeedHeroduplication: fixed.FeedHerousesuseFeedHeroCardProps, and the handlers are identical.- Small top story impression: fixed.
useInViewat 0.5, once per post, matching the carousel. On Next 16,legacyBehaviormerges the childref, so the observer attaches. - Excluded posts and ad slots: fixed. The excluded check now runs before
getAd, next to the pinned check, and the spec covers it.
Contract with daily-api#4446 at 00af28d3e: the API now validates shape only (camelCase kind and size, id up to 64 characters, 12 widgets, 8 KB, unknown fields passed through). The client fits: generated ids are about 25 characters and every kind is camelCase. The client doesn't know about the 8 KB cap. That doesn't matter here, because this PR has no config editor, but #6955 should show the error if a write is rejected. Deploy order is unchanged: the API must be live before #6955 writes anything.
test_shared and the Vercel builds were still running when I looked.
Reviewed by AI.
| (id, config) => | ||
| write( | ||
| storedRef.current.map((widget) => | ||
| widget.id === id ? ({ ...widget, config } as FeedWidget) : widget, |
There was a problem hiding this comment.
Non-blocking question: should a config write keep the stored v?
v labels the shape of config, and per d6749f3 that shape is now the client's. Here this build replaces config with one in its own shape but keeps whatever v was stored. If a newer client saved v: 2, an older client editing the widget saves a v1-shaped config still marked v: 2, and later migrations will trust that label. Nothing reads v yet, so this is cheap to settle now: either set v to this build's version for the kind (absent for 1), or drop it on a config write.
Reviewed by AI.
There was a problem hiding this comment.
Fixed in 653d069. A config write now stamps this build's version for the kind (feedWidgetConfigVersion, left off for 1). A widget whose stored v is newer than this build knows is neither read for render (it falls back to its defaults) nor editable, so its config is never rewritten in an older shape. It can still be moved, resized or removed, since that leaves config and v untouched.
daily-api#4446 now stores any small config object and checks only its shape, so the client is the one place a config's meaning is checked. config.ts holds a Zod schema per kind, typed against FeedWidgetConfigByKind so the two cannot drift. visibleFeedWidgets hands each widget its config only when it validates, with unknown fields stripped; an invalid one, or any config on a kind that takes none, leaves the widget on its defaults instead of crashing it. The stored list keeps the raw config, so a reorder from this client never overwrites a newer client's shape. updateWidgetConfig refuses a config that does not validate and says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Product review, focused on the decisions that get hard to change once #6955 starts writing rails. Stored shape. Let's store the rail as Dynamic default. No layout shift, and it has to be fast. The strip sits above the feed on every main-feed load, so it should paint in its final shape right away:
Different sizes per widget. Each kind already picks its own sizes from Small, Tall and Large, which is the right start. We should keep the model open to sizes beyond those three, so a widget can ship its own shape (a 2x1 strip, a wider Large) without breaking older clients. Two things work against that today:
|
…idgets
Product review follow-ups on the widget rail foundation.
- The stored rail is { version, widgets, ...rail fields } (daily-api
#4446). readStoredFeedRail keeps rail-level fields another client
wrote, and the first edit records `startedFrom`, the default the
reader began with, so a newer default's widgets can be offered later.
- The default rail comes from the feed_widgets_default JSON flag, with
today's rail as its default value, so the composition can be tested
without a deploy and older extension builds stop carrying a stale
one. It is only evaluated for readers who never customized; a value
that reads as no widgets falls back to the built-in.
- Sizes are named spans in feedWidgetSizeCatalog; a new shape is a
catalog entry that kinds opt into in the registry, and spans may be
wider than two columns.
- An older client no longer overwrites sizes it does not know: a widget
keeps its stored size through any reorder, and only the one the
reader resized gets a new size written.
- Widget components are lazy: each is its own chunk, loaded when one
first renders, behind a skeleton.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A rail saved before startedFrom existed was stamped with today's default on its next edit, which is not where that reader started. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The feed page needs to know which kinds a reader's rail holds before its grid paints, on every load. Reading that through useFeedWidgets pulled the registry, the widget map and the Zod config schemas into the feed's bundle. storedRail.ts holds the stored-rail reader, the built-in default and the flag default's resolution, and imports only the types. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
So code that only needs the slot's column does not import the layout module and the config schemas behind it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@idoshamun Thanks, all addressed. Here's where each point landed. Stored shape (dccb435, f1c8311)
Dynamic default (dccb435, 34347fc)
No layout shift and fast (#6955, c41e329). Your rule is the design: anything that changes the grid is known before it paints, or it waits for the next load.
Only on-screen widgets fetch (#6955)
CLS check, local Popular feed, headless Chromium, 1500px:
The 0.0866 was the late rail standing the grid's first ad down. It's in #6955's test plan too. Sizes
Reviewed by AI. |
rebelchris
left a comment
There was a problem hiding this comment.
Re-review of f74586ea3..f04be9953.
- The reorder no longer saves the size shown for display. Fixed in dccb435:
mergeRenderedFeedWidgetskeeps the stored size unless the reader changed it, andlayout.spec.tscovers it. I've resolved that thread, and the four from the first round that were verified last time. - Should a config write keep the stored
v? Still open, with no reply on the thread. The newversionfield raises the same question at the level of the whole rail (inline below), so the two are worth settling together. - Contract with daily-api#4446 (
c79175392): it matches. The rail is stored as{ version, widgets, ... }on both sides. Generated ids, the flag default's ids, and every kind and size fit the API's length and camelCase checks. - No other new findings in the per-widget config schemas, the
feed_widgets_defaultflag, the lazy widget chunks orstoredRail.ts. - Size: this PR has grown to about 1.9k added lines across 22 files, roughly a third of it tests. It is still reviewable, but further product changes would be better as a follow-up PR than added here.
Reviewed by AI.
| // the default records which default this reader started from. | ||
| const rail: FeedWidgetRailState | null = next && { | ||
| ...(savedRef.current ?? { startedFrom: defaultRef.current.id }), | ||
| version: FEED_WIDGET_RAIL_VERSION, |
There was a problem hiding this comment.
Non-blocking question: an older build overwrites a newer rail version with its own.
version is the stored shape's version, so a later client can change what the rail means (the default the reader started from, a hidden rail, what widgets holds). This write always sets version: FEED_WIDGET_RAIL_VERSION (1), and readStoredFeedRail accepts any version as if it were 1.
Example: an older extension build loads a rail a newer webapp saved as version: 2. It reads it as a v1 rail, and its first edit saves version: 1 while keeping the v2-only fields through ...savedRef.current. The version label now says 1 for data that has v2 meaning, which a later migration would trust.
This is cheap to decide now, while nothing has been written yet. Options:
- A build that sees
version > FEED_WIDGET_RAIL_VERSIONrenders what it can, but treats the rail as read-only: no edits, or the editing controls hidden. - Keep the stored version with
Math.max, and document that a version bump must stay readable and writable by older builds.
The open v question on updateWidgetConfig is the same decision for each widget's config, so one rule could cover both.
Reviewed by AI.
There was a problem hiding this comment.
Fixed in 653d069 with option 1. A rail whose version is above FEED_WIDGET_RAIL_VERSION renders what it can but is read-only: isReadOnly, no add tile, and every write refused at the hook (isReadOnlyRef), so an older build can never relabel it. This is the first build that writes, so the rule ships before any rail exists.
…ndation # Conflicts: # packages/shared/src/lib/featureManagement.ts
|
Thanks, the stored shape, default flag and sizes all check out. One hole is left in the stored shape, the one the AI review raised on Every write stamps One rule for both:
Keeping the higher version instead ( |
…alone Product review: every write stamped this build's rail version and readStoredFeedRail read any version as 1, so an older build (an extension that has not updated) editing a rail a newer client saved as v2 would relabel it v1 while keeping the v2-only fields, and a later migration would trust that label. A widget's `v` had the same problem: a config write in this build's shape kept whatever `v` was stored. One rule for both, in the first build that writes, since that build lives on in old extensions and cannot be patched: - A rail whose `version` is above FEED_WIDGET_RAIL_VERSION renders what it can but is read-only: isReadOnly, canAdd false, every write refused. - A widget whose `v` is above this build's config version for its kind (feedWidgetConfigVersions, 1 for all today) can still be moved, resized or removed, but its config is neither read for rendering nor edited. A config write stamps this build's version for the kind, absent for 1, dropping whatever label was stored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The hero query starts from the last payload this reader fetched, so the top story paints with content on the first frame and revalidates in the background. The lead it shows is held for the page session (the live copy by id, so votes still land), the same rule the grid follows, and a fresh lead waits for the next load. useFeed never drops a post the grid already rendered when an exclusion arrives late (a first visit with nothing cached), so nothing on screen moves; the late lead shows twice for that one load at most. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@idoshamun Both points are addressed on this PR. Version rule (653d069)
Hero payload cache (d14a920), for your #6955 point
|
Summary
Part 1 of 2 for the feed widget rail: the hero becomes a row of widgets the reader arranges, with our ad pinned to the third slot (design: #6943, Header A). This PR is the model only. Nothing is wired into a page yet, so it changes no behavior on merge. The UI and feed wiring follow in #6955, stacked on this one.
Storage is dailydotdev/daily-api#4446 (
Settings.flags.feedWidgets).What to review
features/feedWidgets/layout.ts: first-fit packing on a two-row grid (Small = 1 cell, Tall = a column, Large = two columns) with the ad reserved at the third column. A Large counts as two, so after it the ad is the second item. When a Large can't straddle the reserved column and would leave a column before the ad empty, the ad closes up instead. A lone Small's free cell is returned as a gap slot. All cases inlayout.spec.ts.registry.tsx+components.ts: metadata for every kind the API knows (top story, Happening Now, briefing, notifications, messages, squad updates, reading list, topical digest, streak, weather, to-do), and the map of built ones. A stored kind this build lacks is dropped rather than crashing; split in two to avoid an import cycle with the widgets.useFeedWidgets.ts: reads and writesflags.feedWidgets, each write onesetSettingscarrying the whole flags object (updateFlagsnapshots and loses concurrent writes). Default rail when nothing is stored: the top story alone. Undo recomputes against the latest list.storedRail.ts): the rail is{ version, widgets, ...rail fields }. Rail-level fields are kept as stored, and an edit made from the default recordsstartedFrom. The module imports only types, so the feed page can read it without pulling in the widgets.feed_widgets_defaultJSON flag, with today's rail as its default value. It's evaluated only for readers who never customized, and a broken value falls back to the built-in.feedWidgetSizeCatalog, and kinds opt into them in the registry. A widget keeps its stored size through any reorder; only the one the reader resized gets a new size written, so an older client never resets a newer size.config.ts: the client is the one place a widget's config is checked, because daily-api#4446 stores any small object. Each kind has a Zod schema, typed againstFeedWidgetConfigByKind. A widget gets its config only when it validates, with unknown fields stripped. An invalid config leaves the widget on its defaults. The stored list keeps the raw config, so this client never overwrites a newer client's shape.updateWidgetConfigrefuses an invalid config. All of this is covered inconfig.spec.ts.useFeedHeroData, whichFeedHeronow uses too.useFeedHeroData: starts from the last payload this reader fetched (cached per reader), so widgets paint with content and revalidate in the background. It returnsleadPost, the lead held for the page session, which the top story shows and the grid keeps out.versionnewer than this build is read-only. A widgetvnewer than this build's config version for its kind isn't read or edited, and a config write stamps this build's version.useFeed/Feed: newexcludedPostIdsseeds the existing dedupe set, so a post on the rail is kept out of the grid. A post the grid already rendered is never pulled when its exclusion arrives late.Test plan
layout.spec.ts,config.spec.ts,useFeedWidgets.spec.tsx,useFeedHeroData.spec.tsx(cache seed, held lead),useFeed.spec.tsx(late exclusion keeps a rendered post), existing hero specs🤖 Generated with Claude Code
Preview domain
https://feat-feed-widgets-foundation.preview.app.daily.dev