Skip to content

De-cardify UI surfaces and harden GitOps diffs#268

Merged
IgorWarzocha merged 146 commits into
devfrom
de-cardify-ui
May 24, 2026
Merged

De-cardify UI surfaces and harden GitOps diffs#268
IgorWarzocha merged 146 commits into
devfrom
de-cardify-ui

Conversation

@IgorWarzocha
Copy link
Copy Markdown
Owner

Summary

This is the big de-cardification / UI hardening branch.

  • flattens a lot of the app UI: chat/tool rows, composer popovers, Skills/Extensions, Settings, Inbox, artifacts, GitOps diffs, and empty states
  • adds @howcode/* module boundaries and moves native-ish capability surfaces into dedicated folders
  • upgrades GitOps diffs around streamed Pierre CodeView rendering, image previews, safer untracked handling, and sturdier diff comments
  • tightens composer/popover behavior, artifact editing, archive/skills edge cases, and a pile of small review-found correctness bugs

Notes

This deliberately keeps behavior mostly the same while cleaning up the surface grammar and making future pluginisation less painful. Sidebar work is mostly parked and should be handled separately.

Validation

  • bun run ai:check
  • packaged temp build launched and manually sanity-checked through GitOps diffs/comments, image diffs, composer popovers, Skills search, and related UI states
  • multiple scoped review passes were run and fixed before submission

Changelog

Updated docs/changelog.md under 0.1.66.

@reactreview
Copy link
Copy Markdown

reactreview Bot commented May 24, 2026

72 score

Copy as prompt
Check if these React Review issues are valid. If so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.

Run this before and after your changes to verify the result:
npx react-doctor@latest --verbose --diff

Do not modify the react-doctor configuration unless explicitly asked.
Fix the underlying code issues instead of changing or suppressing the rules.

React Review found 0 errors and 282 warnings. This PR leaves the React health score unchanged.

<file name="src/app/chat-workspace/chat-workspace-composer.tsx">

<violation number="1" location="src/app/chat-workspace/chat-workspace-composer.tsx:107">
Severity: Warning

w-8 h-8 → use the shorthand size-8 (Tailwind v3.4+)

Collapse `w-N h-N` to `size-N` (Tailwind v3.4+) when both axes match

Rule: `design-no-redundant-size-axes`
</violation>

<violation number="2" location="src/app/chat-workspace/chat-workspace-composer.tsx:271">
Severity: Warning

JSX prop receives JSX created on every render — extract it or memoize to avoid re-renders.

Hoist the inner JSX outside the render or memoize via `useMemo`.

Rule: `jsx-no-jsx-as-prop`
</violation>

</file>

<file name="src/app/composer/composer-context-meter.tsx">

<violation number="1" location="src/app/composer/composer-context-meter.tsx:279">
Severity: Warning

Avoid adjusting state when a prop changes. Instead, adjust the state directly during render, or refactor your state to avoid this need entirely.

Adjust the state inline during render instead of via a useEffect, or refactor the state to avoid the need entirely. See https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes

Rule: `no-adjust-state-on-prop-change`
</violation>

<violation number="2" location="src/app/composer/composer-context-meter.tsx:280">
Severity: Warning

Avoid adjusting state when a prop changes. Instead, adjust the state directly during render, or refactor your state to avoid this need entirely.

Adjust the state inline during render instead of via a useEffect, or refactor the state to avoid the need entirely. See https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes

Rule: `no-adjust-state-on-prop-change`
</violation>

<violation number="3" location="src/app/composer/composer-context-meter.tsx:285">
Severity: Warning

Avoid chaining state changes. When possible, update all relevant state simultaneously.

Update all related state simultaneously inside the event handler that originally fires, instead of reacting to one state update in a useEffect that writes another state. See https://react.dev/learn/you-might-not-need-an-effect#chains-of-computations

Rule: `no-chain-state-updates`
</violation>

<violation number="4" location="src/app/composer/composer-context-meter.tsx:285">
Severity: Warning

Avoid storing derived state. Compute "usageTotals" directly during render, optionally with `useMemo` if it's expensive.

Compute derived state inline during render (or with useMemo if expensive) instead of mirroring it into useState via a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state

Rule: `no-derived-state`
</violation>

<violation number="5" location="src/app/composer/composer-context-meter.tsx:212">
Severity: Warning

Avoid using state and effects as an event handler. Instead, call the event handling code directly when the event occurs.

Move the side effect into the event handler that triggers it, instead of guarding on its state inside a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#sharing-logic-between-event-handlers

Rule: `no-event-handler`
</violation>

<violation number="6" location="src/app/composer/composer-context-meter.tsx:212">
Severity: Warning

Avoid using state and effects as an event handler. Instead, call the event handling code directly when the event occurs.

Move the side effect into the event handler that triggers it, instead of guarding on its state inside a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#sharing-logic-between-event-handlers

Rule: `no-event-handler`
</violation>

<violation number="7" location="src/app/composer/composer-context-meter.tsx:299">
Severity: Warning

w-7 h-7 → use the shorthand size-7 (Tailwind v3.4+)

Collapse `w-N h-N` to `size-N` (Tailwind v3.4+) when both axes match

Rule: `design-no-redundant-size-axes`
</violation>

</file>

<file name="src/app/composer/composer-file-picker-attachments-panel.tsx">

<violation number="1" location="src/app/composer/composer-file-picker-attachments-panel.tsx:78">
Severity: Warning

w-4 h-4 → use the shorthand size-4 (Tailwind v3.4+)

Collapse `w-N h-N` to `size-N` (Tailwind v3.4+) when both axes match

Rule: `design-no-redundant-size-axes`
</violation>

<violation number="2" location="src/app/composer/composer-file-picker-attachments-panel.tsx:89">
Severity: Warning

w-3.5 h-3.5 → use the shorthand size-3.5 (Tailwind v3.4+)

Collapse `w-N h-N` to `size-N` (Tailwind v3.4+) when both axes match

Rule: `design-no-redundant-size-axes`
</violation>

</file>

<file name="src/app/composer/composer-dictation-controls.tsx">

<violation number="1" location="src/app/composer/composer-dictation-controls.tsx:165">
Severity: Warning

Avoid adjusting state when a prop changes. Instead, adjust the state directly during render, or refactor your state to avoid this need entirely.

Adjust the state inline during render instead of via a useEffect, or refactor the state to avoid the need entirely. See https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes

Rule: `no-adjust-state-on-prop-change`
</violation>

<violation number="2" location="src/app/composer/composer-dictation-controls.tsx:164">
Severity: Warning

Avoid using props and effects as an event handler. Instead, move the handler to the parent component.

Move the side effect into the event handler that triggers it, instead of guarding on its state inside a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#sharing-logic-between-event-handlers

Rule: `no-event-handler`
</violation>

<violation number="3" location="src/app/composer/composer-dictation-controls.tsx:164">
Severity: Warning

Avoid using props and effects as an event handler. Instead, move the handler to the parent component.

Move the side effect into the event handler that triggers it, instead of guarding on its state inside a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#sharing-logic-between-event-handlers

Rule: `no-event-handler`
</violation>

<violation number="4" location="src/app/composer/composer-dictation-controls.tsx:163">
Severity: Warning

Avoid resetting all state when a prop changes. Instead, if "dictationMissingModel" is a key, pass it as `key` so React will reset the component's state.

Pass the prop as `key` so React resets the component when the prop changes, instead of manually resetting every state value in a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#resetting-all-state-when-a-prop-changes

Rule: `no-reset-all-state-on-prop-change`
</violation>

</file>

Showing 15 of 282 issues.

Reviewed by reactreview for commit c36f40b. Configure here.

@IgorWarzocha
Copy link
Copy Markdown
Owner Author

@codex please review this PR and give me 10-20 issues if any. Categorize findings as required, recommended, or optional.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3eb6275a51

ℹ️ 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".

Comment thread src/app/hooks/useDesktopDiff.ts Outdated
Comment on lines +45 to +49
return queryClient.getQueryCache().subscribe((event) => {
const queryKey = event.query.queryKey
if (!isCurrentProjectDiffQuery(queryKey, projectId)) return
if (event.type === 'removed' || event.query.state.isInvalidated) {
setVersion((current) => current + 1)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge [required] Refresh diff stream when projectDiff cache is absent

useProjectDiffStream relies on useProjectDiffInvalidationVersion to restart streaming after worktree-invalidating actions, but the version bump only happens from QueryCache events for existing projectDiff queries. In streaming mode the projectDiff query is disabled (queryEnabled is false), so that cache entry often does not exist; invalidation calls then produce no matching event and the stream never restarts. This leaves the GitOps panel showing stale diff content after commits/staging changes until the user manually changes baseline/view.

Useful? React with 👍 / 👎.

Comment thread src/app/hooks/useDesktopDiff.ts Outdated
includeUntracked,
streamRefreshVersion,
)
const queryEnabled = enabled && Boolean(projectId) && !stream.canStream
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge [recommended] Fallback to non-stream diff when stream startup fails

The hook disables the legacy query path whenever the streaming API method exists, even if starting the stream fails at runtime (IPC mismatch, transient backend error, or bridge issue). In that case users only see an error/null diff although getProjectDiff could still return usable data. Adding a fallback to the non-stream query on stream start failure would prevent complete diff outage in partial-failure environments.

Useful? React with 👍 / 👎.

@IgorWarzocha
Copy link
Copy Markdown
Owner Author

Addressed the required streamed-diff refresh finding. Streaming diffs now use an explicit project diff invalidation signal instead of relying on a disabled React Query cache entry existing. Also removed the old non-stream project diff request/query path rather than adding fallback behavior; streamed diffs are the only renderer path now.

@IgorWarzocha
Copy link
Copy Markdown
Owner Author

@codex please review this PR and give me 10-20 issues if any. Categorize findings as required, recommended, or optional.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b4d6e50aa

ℹ️ 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".

baselineRev,
env,
hasHead,
includeUntracked: true,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep tracked-only stats when untracked files are excluded

In loadTrackedWorktreeSnapshot, the patch is generated from git add -u (tracked files only), but stats are computed with includeUntracked: true, so fileCount/insertions/deletions can include untracked files that are not present in the emitted diff. This produces inconsistent metadata (e.g., header counts disagree with rendered patch) whenever untracked files exist and includeUntracked is disabled.

Useful? React with 👍 / 👎.

diffBaseline,
showDiffInMainView && (projectGitState?.isGitRepo ?? false),
)
const diffLoadError: string | null = null
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore diff error propagation to the GitOps footer

This sets diffLoadError to a constant null, removing the previous wiring from useDesktopDiff and preventing diff-load failures from surfacing in the composer GitOps message field. When diff streaming/parsing fails, users no longer get the footer-level error signal that was previously shown, which makes comment/commit flows appear healthy even though diff data is unavailable.

Useful? React with 👍 / 👎.

Comment thread src/app/hooks/useHoverToFocus.ts Outdated
blurOnLeave = false,
tolerancePx = DEFAULT_HOVER_TOLERANCE_PX,
isFocused,
extraBoundaryRefs = [],
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Stabilize default extra boundary refs in hover hook

Using extraBoundaryRefs = [] in the parameter list creates a new array on every render, and that array is included in the effect dependency list, so the pointermove listener is torn down and re-registered every render when callers omit this prop. This adds avoidable churn to a global event listener and can degrade hover/focus responsiveness in frequently re-rendering views.

Useful? React with 👍 / 👎.

Comment thread package.json Outdated
"check": "bun run lint && bun run typecheck && bun run test && bun run build",
"ai:check": "bun run lint && bun run typecheck && bun run test",
"check": "bun run ai:check && bun run build",
"ai:check": "biome check --write --error-on-warnings . && tsgo --noEmit && tsgo -p tsconfig.desktop.json --noEmit && tsgo -p tsconfig.electron.json --noEmit && tsgo -p tsconfig.scripts.json --noEmit && bun ./scripts/check-vite-import-resolution.ts && vitest run",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid auto-writing files inside ai:check

ai:check now runs biome check --write, and .husky/pre-push executes ai:check, so the pre-push gate mutates tracked files after commits are created. That means push validation can run against locally rewritten code that is not in the commit being pushed, leaving dirty trees and allowing unformatted/unchecked commit contents to be pushed despite a passing hook.

Useful? React with 👍 / 👎.

@IgorWarzocha
Copy link
Copy Markdown
Owner Author

Addressed this batch too:\n\n- fixed tracked-only diff stats so hidden untracked files no longer affect visible diff counts\n- restored diff load error propagation from the streamed diff panel up to the GitOps footer\n- stabilized the hover-to-focus default boundary refs\n- made bun run ai:check non-mutating again, while pre-push now runs Biome write explicitly before the non-mutating check\n\nValidation passed via bun run ai:check and the push hook.

@IgorWarzocha IgorWarzocha merged commit 36ef547 into dev May 24, 2026
1 check passed
@IgorWarzocha IgorWarzocha deleted the de-cardify-ui branch May 24, 2026 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant