Skip to content

[FRONTEND] Replace AppShell 100ms localStorage polling with event-based sidebar sync#715

Closed
Pcmhacker-piro wants to merge 4 commits into
utksh1:mainfrom
Pcmhacker-piro:sidebar-event-sync
Closed

[FRONTEND] Replace AppShell 100ms localStorage polling with event-based sidebar sync#715
Pcmhacker-piro wants to merge 4 commits into
utksh1:mainfrom
Pcmhacker-piro:sidebar-event-sync

Conversation

@Pcmhacker-piro

Copy link
Copy Markdown
Contributor

✦ Description

Replaced the 100ms setInterval polling loop in AppShell that was continuously reading localStorage to detect sidebar expansion state changes. Instead, the Sidebar component now dispatches a sidebar-state-changed CustomEvent whenever its expanded state changes, and AppShell listens for this event.

Changes Made

  • Sidebar.tsx: Dispatch a sidebar-state-changed CustomEvent on window whenever isExpanded changes (alongside the existing localStorage write for cross-tab persistence).
  • AppShell.tsx:
    • Removed the 100ms setInterval polling loop
    • Removed the storage event listener (only works across tabs, not same-tab)
    • Added listener for the sidebar-state-changed CustomEvent for same-tab synchronization

Why

The previous approach used a setInterval that ran every 100ms to poll localStorage, which is wasteful and brittle. The new approach uses explicit event-based communication that only fires when the state actually changes.

Fixes #553

⟡ Type of Change

  • Refactor / Performance improvement

✦ Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • My changes generate no new warnings or errors
  • Sidebar state stays synchronized across the shell
  • Existing persistence behavior still works

OpenCode Agent added 2 commits June 9, 2026 10:44
…ecycle

- Add workflow.spec.ts with mocked API routes for GET/POST/DELETE /workflows
- Cover creation via CreateSheet, manual run with queued task display,
  and delete with grid removal confirmation
- Include full lifecycle scenario combining create, run, and delete
- Set up API key in localStorage before navigation to bypass auth gate

Fixes utksh1#563
…sed sidebar sync

- Remove 100ms polling interval from AppShell that was polling
  localStorage to detect sidebar state changes
- Sidebar now dispatches a 'sidebar-state-changed' CustomEvent on
  window whenever isExpanded changes
- AppShell listens for the CustomEvent instead of polling
- Preserves localStorage persistence for cross-tab scenarios
- No external dependencies or context providers needed
@Pcmhacker-piro

Copy link
Copy Markdown
Contributor Author

heyy @utksh1
i fixed the issue so pls check it

@utksh1 utksh1 added level:intermediate 35 pts difficulty label for moderate contributor PRs type:bug Bug fix work category bonus label type:refactor Refactor work category bonus label area:frontend Frontend React/UI work labels Jun 9, 2026

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This needs changes before merge. The PR is stacked on the workflow E2E PR and includes unrelated test additions. More importantly, replacing the storage listener with only a custom event loses cross-tab localStorage synchronization. Please split/rebase this to only the sidebar sync change and keep both same-tab custom event handling and cross-tab storage handling covered.

OpenCode Agent and others added 2 commits June 10, 2026 14:05
- Keep storage event listener (browser-native cross-tab synchronization)
- Keep CustomEvent listener (same-tab updates via Sidebar)
- Remove 100ms setInterval polling (not needed)
- Remove unrelated e2e workflow.spec.ts test additions
@Pcmhacker-piro

Copy link
Copy Markdown
Contributor Author

heyy @utksh1
i fixed the issue so pls check it

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Re-reviewed the latest update. This is closer, but still not merge-ready.

The diff still appears stacked with unrelated workflow E2E history, and the AppShell formatting is visibly off after the change. Please rebase/split so only AppShell/Sidebar sidebar sync changes remain, fix formatting, and add focused tests for both same-tab custom-event sync and cross-tab storage sync.

@Pcmhacker-piro Pcmhacker-piro deleted the sidebar-event-sync branch June 12, 2026 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:frontend Frontend React/UI work level:intermediate 35 pts difficulty label for moderate contributor PRs type:bug Bug fix work category bonus label type:refactor Refactor work category bonus label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FRONTEND] Replace AppShell 100ms localStorage polling with event- or context-based sidebar sync

2 participants