Polish source health command center - #24
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the source health monitoring UI by introducing a dedicated SourceHealthActions component and redesigning the SourceHealthCard to distinguish between synchronization health and reading readiness. It also implements a 'Next Action' recommendation system and updates the documentation to reflect these UI patterns. Feedback includes addressing potential hydration mismatches caused by using new Date() in the component body, replacing magic numbers for time durations with named constants, and removing redundant logic in the indicator deduplication set.
| style, | ||
| indicatorLimit = 4, | ||
| }: SourceHealthCardProps) { | ||
| const now = new Date(); |
There was a problem hiding this comment.
Initializing now with new Date() directly in the component body can lead to hydration mismatches in SSR environments. If the server and client render times differ slightly, the relative date strings generated by formatRelativeDate might not match exactly between the server-rendered HTML and the client's first render, causing React to throw a hydration warning. Consider using a hook to ensure the value is stable or only calculated on the client after mount.
| snoozed_until: isSnoozed ? null : new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(), | ||
| }) | ||
| } | ||
| > | ||
| {isSnoozed ? "Usuń drzemkę" : "Wstrzymaj 1d"} | ||
| </WorkspaceButton> | ||
| <WorkspaceButton | ||
| aria-label={isPaused ? `Wznów źródło: ${entry.title}` : `Pauza 7 dni: ${entry.title}`} | ||
| disabled={workspaceBusy} | ||
| onClick={() => | ||
| void onControlUpdate(entry.channel_id, { | ||
| paused_until: isPaused ? null : new Date(Date.now() + 7 * 24 * 60 * 60 * 1000).toISOString(), |
| const controlGroupName = source.control?.groupName; | ||
| const indicatorDedupe = new Set( | ||
| [ | ||
| source.control?.tier, | ||
| source.control?.groupName, | ||
| controlGroupName, | ||
| "bez grupy", | ||
| ].filter(Boolean), | ||
| ); |
There was a problem hiding this comment.
The indicatorDedupe set construction contains redundant entries. source.control?.groupName and controlGroupName refer to the same value, and both are included in the array. You can simplify this by removing the intermediate variable and the duplicate entry.
const indicatorDedupe = new Set(
[
source.control?.tier,
source.control?.groupName,
"bez grupy",
].filter(Boolean),
);There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a706c1b233
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const isPaused = Boolean(entry.control.paused_until); | ||
| const isSnoozed = Boolean(entry.control.snoozed_until); |
There was a problem hiding this comment.
Determine timer state from future timestamps
isPaused/isSnoozed are derived from string presence, so any historical paused_until or snoozed_until value is treated as active. In this codebase, suppression logic only considers timers active when they are in the future (> now), so expired timers remain persisted but should not flip the UI into resume/unsnooze mode. With the current check, cards can show Wznów/Usuń drzemkę for already-expired timers and hide the expected one-click Pauza 7d/Wstrzymaj 1d actions.
Useful? React with 👍 / 👎.
Summary
Validation
Evidence
Closes #12