From 6507b3646c603ca9fff22ff9cd1bd69fc4ff2f9b Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 11 Apr 2026 09:23:52 +0000 Subject: [PATCH 1/2] Fix issue #17: Fokus Task is not kept in local storage --- FOCUS_FIX_DOCUMENTATION.md | 77 ++++++++++++ src/app/shared/focus.service.spec.ts | 59 +++++++++- src/app/shared/focus.service.ts | 20 +++- test_focus_logic.html | 167 +++++++++++++++++++++++++++ test_focus_logic.js | 163 ++++++++++++++++++++++++++ 5 files changed, 484 insertions(+), 2 deletions(-) create mode 100644 FOCUS_FIX_DOCUMENTATION.md create mode 100644 test_focus_logic.html create mode 100644 test_focus_logic.js diff --git a/FOCUS_FIX_DOCUMENTATION.md b/FOCUS_FIX_DOCUMENTATION.md new file mode 100644 index 0000000..00da8da --- /dev/null +++ b/FOCUS_FIX_DOCUMENTATION.md @@ -0,0 +1,77 @@ + +# Focus Service Local Storage Fix + +## Problem +When a focused task or work item was selected and the page was refreshed, the focus was lost. This happened because the focus service was clearing the focus target when the focused item couldn't be resolved, even if the data wasn't loaded yet. + +## Root Cause +The original implementation had an effect that would clear the focus whenever the focused item couldn't be resolved: + +```typescript +effect(() => { + const item = this.focusedItem(); + const target = this.focusTarget(); + if (target && !item) { + this.focusTarget.set(null); // This would clear focus immediately + } +}); +``` + +During page refresh: +1. FocusService loads focus target from local storage +2. Effect runs and tries to resolve the item +3. Data services (Jira, Bitbucket) haven't loaded data yet, so arrays are empty +4. Focused item can't be found, so focus is cleared +5. User loses their focus state + +## Solution +Modified the effect to check if data has been loaded before clearing the focus: + +```typescript +effect(() => { + const item = this.focusedItem(); + const target = this.focusTarget(); + if (target && !item) { + // Only clear focus if data has been loaded and item still doesn't exist + const dataLoaded = this.isDataLoadedForTarget(target); + if (dataLoaded) { + this.focusTarget.set(null); + } + } +}); +``` + +Added a new method `isDataLoadedForTarget(target)` that checks the loading state for each data type: +- **Tickets**: Check `!this.data.ticketsLoading()` +- **Pull Requests**: Check `!this.data.pullRequestsLoading()` +- **Todos/Ideas**: Always return `true` (data is available immediately from local storage) + +## Behavior After Fix + +### Tickets and Pull Requests +1. User sets focus on a ticket/PR +2. Page refreshes +3. Focus is loaded from local storage +4. Data is still loading → focus is preserved +5. Data finishes loading: + - If item exists → focus remains + - If item doesn't exist → focus is cleared + +### Todos and Ideas +1. User sets focus on a todo/idea +2. Page refreshes +3. Focus is loaded from local storage +4. Data is immediately available: + - If item exists → focus remains + - If item doesn't exist → focus is cleared immediately + +## Files Modified +- `/workspace/src/app/shared/focus.service.ts`: Added data loading check to focus validation effect +- `/workspace/src/app/shared/focus.service.spec.ts`: Added tests for the new behavior + +## Testing +The fix ensures that: +- Focus is preserved during data loading for tickets and PRs +- Focus is cleared appropriately when items no longer exist after data is loaded +- Todos and ideas continue to work as before +- Local storage persistence continues to work correctly diff --git a/src/app/shared/focus.service.spec.ts b/src/app/shared/focus.service.spec.ts index 6ae3dec..e7fb16b 100644 --- a/src/app/shared/focus.service.spec.ts +++ b/src/app/shared/focus.service.spec.ts @@ -56,6 +56,8 @@ describe('FocusService', () => { const ideasSignal = signal([makeIdea('id-1')]); const ticketsSignal = signal([makeTicket('tk-1')]); const pullRequestsSignal = signal([]); + const ticketsLoadingSignal = signal(false); + const pullRequestsLoadingSignal = signal(false); beforeEach(() => { localStorage.clear(); @@ -63,6 +65,8 @@ describe('FocusService', () => { ideasSignal.set([makeIdea('id-1')]); ticketsSignal.set([makeTicket('tk-1')]); pullRequestsSignal.set([]); + ticketsLoadingSignal.set(false); + pullRequestsLoadingSignal.set(false); TestBed.configureTestingModule({ providers: [ FocusService, @@ -70,7 +74,12 @@ describe('FocusService', () => { { provide: IdeaService, useValue: { ideas: ideasSignal } }, { provide: WorkspaceService, - useValue: { tickets: ticketsSignal, pullRequests: pullRequestsSignal }, + useValue: { + tickets: ticketsSignal, + pullRequests: pullRequestsSignal, + ticketsLoading: ticketsLoadingSignal, + pullRequestsLoading: pullRequestsLoadingSignal + }, }, ], }); @@ -133,4 +142,52 @@ describe('FocusService', () => { JSON.stringify({ id: 'td-1', type: 'todo' }), ); }); + + it('preserves focus when data is not loaded yet', () => { + // Set focus on a ticket + service.setFocus({ id: 'tk-1', type: 'ticket' }); + TestBed.tick(); + + // Verify focus is set + expect(service.focusTarget()).toEqual({ id: 'tk-1', type: 'ticket' }); + expect(service.focusedItem()).toEqual(makeTicket('tk-1')); + + // Simulate data being cleared (but loading is complete) + ticketsSignal.set([]); + TestBed.tick(); + + // Focus should be cleared because data is loaded and item doesn't exist + expect(service.focusTarget()).toBeNull(); + + // Now simulate the actual issue: focus loaded from storage but data not loaded yet + ticketsLoadingSignal.set(true); + + // Set focus again and clear data + service.setFocus({ id: 'tk-1', type: 'ticket' }); + ticketsSignal.set([]); + TestBed.tick(); + + // Focus should be preserved because data is still loading + expect(service.focusTarget()).toEqual({ id: 'tk-1', type: 'ticket' }); + expect(service.focusedItem()).toBeNull(); + + // Now mark data as loaded + ticketsLoadingSignal.set(false); + TestBed.tick(); + + // Focus should be cleared because data is loaded and item doesn't exist + expect(service.focusTarget()).toBeNull(); + }); + + it('clears focus when item no longer exists after data is loaded', () => { + service.setFocus({ id: 'tk-1', type: 'ticket' }); + TestBed.tick(); + + // Remove the ticket from data + ticketsSignal.set([]); + TestBed.tick(); + + // Focus should be cleared because data is loaded and item doesn't exist + expect(service.focusTarget()).toBeNull(); + }); }); diff --git a/src/app/shared/focus.service.ts b/src/app/shared/focus.service.ts index 4c04f0c..dddd210 100644 --- a/src/app/shared/focus.service.ts +++ b/src/app/shared/focus.service.ts @@ -25,7 +25,11 @@ export class FocusService { const item = this.focusedItem(); const target = this.focusTarget(); if (target && !item) { - this.focusTarget.set(null); + // Only clear focus if data has been loaded and item still doesn't exist + const dataLoaded = this.isDataLoadedForTarget(target); + if (dataLoaded) { + this.focusTarget.set(null); + } } }); @@ -64,6 +68,20 @@ export class FocusService { } } + private isDataLoadedForTarget(target: FocusTarget): boolean { + switch (target.type) { + case 'ticket': + return !this.data.ticketsLoading(); + case 'pr': + return !this.data.pullRequestsLoading(); + case 'todo': + case 'idea': + // Todo and Idea data is available immediately from local storage + return true; + } + return true; + } + private loadFromStorage(): FocusTarget | null { try { const raw = localStorage.getItem(STORAGE_KEY); diff --git a/test_focus_logic.html b/test_focus_logic.html new file mode 100644 index 0000000..6b81bef --- /dev/null +++ b/test_focus_logic.html @@ -0,0 +1,167 @@ + + + + + Focus Service Logic Test + + + +

Focus Service Logic Test

+ +
+

Test Scenario

+

This test simulates the focus service behavior when:

+
    +
  1. User sets focus on a ticket
    2. +
  2. Page refreshes (data not loaded yet)
    3. +
  3. Data loads
    4. +
  4. Ticket is removed from data
    5. +
+
+ +
+

Test 1: Focus preserved during data loading

+

Expected: Focus should be preserved when data is loading

+

+
+ +
+

Test 2: Focus cleared when item doesn't exist after loading

+

Expected: Focus should be cleared when data is loaded but item doesn't exist

+

+
+ +
+

Test 3: Todo/Idea focus always preserved

+

Expected: Focus on todos/ideas should always be preserved (no loading state)

+

+
+ + + + diff --git a/test_focus_logic.js b/test_focus_logic.js new file mode 100644 index 0000000..7a058e5 --- /dev/null +++ b/test_focus_logic.js @@ -0,0 +1,163 @@ + +#!/usr/bin/env node + +// Simple test to verify the focus service logic +console.log('Testing Focus Service Logic Fix'); +console.log('================================'); + +class MockFocusService { + constructor() { + this.focusTarget = null; + this.ticketsLoading = false; + this.pullRequestsLoading = false; + this.tickets = []; + this.pullRequests = []; + this.todos = []; + this.ideas = []; + } + + setFocus(target) { + this.focusTarget = target; + console.log(`✓ Focus set to: ${JSON.stringify(target)}`); + } + + clearFocus() { + this.focusTarget = null; + console.log('✓ Focus cleared'); + } + + focusedItem() { + if (!this.focusTarget) return null; + return this._resolve(this.focusTarget); + } + + checkFocusValidity() { + console.log(` Current focus: ${JSON.stringify(this.focusTarget)}`); + console.log(` Focused item: ${this.focusedItem() ? 'found' : 'not found'}`); + console.log(` Tickets loading: ${this.ticketsLoading}`); + console.log(` PRs loading: ${this.pullRequestsLoading}`); + + if (this.focusTarget && !this.focusedItem()) { + const dataLoaded = this._isDataLoadedForTarget(this.focusTarget); + console.log(` Data loaded for target: ${dataLoaded}`); + + if (dataLoaded) { + this.clearFocus(); + return 'Focus cleared - data loaded but item not found'; + } else { + return 'Focus preserved - data still loading'; + } + } + return 'Focus valid'; + } + + _resolve(target) { + switch (target.type) { + case 'ticket': + return this.tickets.find(t => t.id === target.id) || null; + case 'pr': + return this.pullRequests.find(p => p.id === target.id) || null; + case 'todo': + return this.todos.find(t => t.id === target.id) || null; + case 'idea': + return this.ideas.find(i => i.id === target.id) || null; + } + return null; + } + + _isDataLoadedForTarget(target) { + switch (target.type) { + case 'ticket': + return !this.ticketsLoading; + case 'pr': + return !this.pullRequestsLoading; + case 'todo': + case 'idea': + return true; // Always available + } + return true; + } +} + +function runTest(testName, testFn) { + console.log(`\n🧪 ${testName}`); + console.log('------------------------------'); + try { + testFn(); + console.log('✅ PASS'); + } catch (error) { + console.log(`❌ FAIL: ${error.message}`); + } +} + +function testFocusPreservedDuringLoading() { + const service = new MockFocusService(); + + // Setup: User sets focus on a ticket + service.tickets = [{ id: 'tk-1', type: 'ticket', title: 'Test Ticket' }]; + service.setFocus({ id: 'tk-1', type: 'ticket' }); + + // Simulate page refresh - data not loaded yet + service.tickets = []; + service.ticketsLoading = true; + + const result = service.checkFocusValidity(); + + if (result !== 'Focus preserved - data still loading') { + throw new Error(`Expected "Focus preserved - data still loading", got "${result}"`); + } + + if (service.focusTarget === null) { + throw new Error('Focus should not be null when data is still loading'); + } +} + +function testFocusClearedWhenItemMissing() { + const service = new MockFocusService(); + + // Setup: User sets focus on a ticket + service.tickets = [{ id: 'tk-1', type: 'ticket', title: 'Test Ticket' }]; + service.setFocus({ id: 'tk-1', type: 'ticket' }); + + // Data loads but ticket is gone + service.tickets = []; + service.ticketsLoading = false; + + const result = service.checkFocusValidity(); + + if (result !== 'Focus cleared - data loaded but item not found') { + throw new Error(`Expected "Focus cleared - data loaded but item not found", got "${result}"`); + } + + if (service.focusTarget !== null) { + throw new Error('Focus should be null when data is loaded but item doesn\'t exist'); + } +} + +function testTodoFocusAlwaysPreserved() { + const service = new MockFocusService(); + + // Setup: User sets focus on a todo + service.todos = [{ id: 'td-1', type: 'todo', title: 'Test Todo' }]; + service.setFocus({ id: 'td-1', type: 'todo' }); + + // Todos are cleared (but no loading state for todos) + service.todos = []; + + const result = service.checkFocusValidity(); + + if (result !== 'Focus cleared - data loaded but item not found') { + throw new Error(`Expected "Focus cleared - data loaded but item not found", got "${result}"`); + } + + if (service.focusTarget !== null) { + throw new Error('Focus should be null when todo doesn\'t exist'); + } +} + +// Run all tests +runTest('Focus preserved during data loading', testFocusPreservedDuringLoading); +runTest('Focus cleared when item missing after loading', testFocusClearedWhenItemMissing); +runTest('Todo focus cleared when item missing', testTodoFocusAlwaysPreserved); + +console.log('\n🎉 All tests completed!'); From 3fa5dee861dd55bdacdfa15f514548a50c2d2c55 Mon Sep 17 00:00:00 2001 From: Dominik Halfkann Date: Sat, 11 Apr 2026 12:13:22 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20clean=20up=20PR=20=E2=80=94=20remove?= =?UTF-8?q?=20junk=20files,=20comments,=20and=20dead=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove FOCUS_FIX_DOCUMENTATION.md, test_focus_logic.html, test_focus_logic.js - Remove explanatory comments per AGENTS.md coding standards - Remove unreachable return after exhaustive switch - Add .openhands_instructions to enforce project rules for OpenHands Co-Authored-By: Claude Opus 4.6 (1M context) --- .openhands_instructions | 3 + FOCUS_FIX_DOCUMENTATION.md | 77 --------------- src/app/shared/focus.service.ts | 10 +- test_focus_logic.html | 167 -------------------------------- test_focus_logic.js | 163 ------------------------------- 5 files changed, 5 insertions(+), 415 deletions(-) create mode 100644 .openhands_instructions delete mode 100644 FOCUS_FIX_DOCUMENTATION.md delete mode 100644 test_focus_logic.html delete mode 100644 test_focus_logic.js diff --git a/.openhands_instructions b/.openhands_instructions new file mode 100644 index 0000000..b384155 --- /dev/null +++ b/.openhands_instructions @@ -0,0 +1,3 @@ +Read and follow `/AGENTS.md` — it is the single source of truth for project rules, coding standards, and design constraints. + +Do not commit documentation files, standalone test scripts, or debugging artifacts. Only commit production code and its corresponding spec files. diff --git a/FOCUS_FIX_DOCUMENTATION.md b/FOCUS_FIX_DOCUMENTATION.md deleted file mode 100644 index 00da8da..0000000 --- a/FOCUS_FIX_DOCUMENTATION.md +++ /dev/null @@ -1,77 +0,0 @@ - -# Focus Service Local Storage Fix - -## Problem -When a focused task or work item was selected and the page was refreshed, the focus was lost. This happened because the focus service was clearing the focus target when the focused item couldn't be resolved, even if the data wasn't loaded yet. - -## Root Cause -The original implementation had an effect that would clear the focus whenever the focused item couldn't be resolved: - -```typescript -effect(() => { - const item = this.focusedItem(); - const target = this.focusTarget(); - if (target && !item) { - this.focusTarget.set(null); // This would clear focus immediately - } -}); -``` - -During page refresh: -1. FocusService loads focus target from local storage -2. Effect runs and tries to resolve the item -3. Data services (Jira, Bitbucket) haven't loaded data yet, so arrays are empty -4. Focused item can't be found, so focus is cleared -5. User loses their focus state - -## Solution -Modified the effect to check if data has been loaded before clearing the focus: - -```typescript -effect(() => { - const item = this.focusedItem(); - const target = this.focusTarget(); - if (target && !item) { - // Only clear focus if data has been loaded and item still doesn't exist - const dataLoaded = this.isDataLoadedForTarget(target); - if (dataLoaded) { - this.focusTarget.set(null); - } - } -}); -``` - -Added a new method `isDataLoadedForTarget(target)` that checks the loading state for each data type: -- **Tickets**: Check `!this.data.ticketsLoading()` -- **Pull Requests**: Check `!this.data.pullRequestsLoading()` -- **Todos/Ideas**: Always return `true` (data is available immediately from local storage) - -## Behavior After Fix - -### Tickets and Pull Requests -1. User sets focus on a ticket/PR -2. Page refreshes -3. Focus is loaded from local storage -4. Data is still loading → focus is preserved -5. Data finishes loading: - - If item exists → focus remains - - If item doesn't exist → focus is cleared - -### Todos and Ideas -1. User sets focus on a todo/idea -2. Page refreshes -3. Focus is loaded from local storage -4. Data is immediately available: - - If item exists → focus remains - - If item doesn't exist → focus is cleared immediately - -## Files Modified -- `/workspace/src/app/shared/focus.service.ts`: Added data loading check to focus validation effect -- `/workspace/src/app/shared/focus.service.spec.ts`: Added tests for the new behavior - -## Testing -The fix ensures that: -- Focus is preserved during data loading for tickets and PRs -- Focus is cleared appropriately when items no longer exist after data is loaded -- Todos and ideas continue to work as before -- Local storage persistence continues to work correctly diff --git a/src/app/shared/focus.service.ts b/src/app/shared/focus.service.ts index dddd210..e468dd2 100644 --- a/src/app/shared/focus.service.ts +++ b/src/app/shared/focus.service.ts @@ -24,12 +24,8 @@ export class FocusService { effect(() => { const item = this.focusedItem(); const target = this.focusTarget(); - if (target && !item) { - // Only clear focus if data has been loaded and item still doesn't exist - const dataLoaded = this.isDataLoadedForTarget(target); - if (dataLoaded) { - this.focusTarget.set(null); - } + if (target && !item && this.isDataLoadedForTarget(target)) { + this.focusTarget.set(null); } }); @@ -76,10 +72,8 @@ export class FocusService { return !this.data.pullRequestsLoading(); case 'todo': case 'idea': - // Todo and Idea data is available immediately from local storage return true; } - return true; } private loadFromStorage(): FocusTarget | null { diff --git a/test_focus_logic.html b/test_focus_logic.html deleted file mode 100644 index 6b81bef..0000000 --- a/test_focus_logic.html +++ /dev/null @@ -1,167 +0,0 @@ - - - - - Focus Service Logic Test - - - -

Focus Service Logic Test

- -
-

Test Scenario

-

This test simulates the focus service behavior when:

-
    -
  1. User sets focus on a ticket
    2. -
  2. Page refreshes (data not loaded yet)
    3. -
  3. Data loads
    4. -
  4. Ticket is removed from data
    5. -
-
- -
-

Test 1: Focus preserved during data loading

-

Expected: Focus should be preserved when data is loading

-

-
- -
-

Test 2: Focus cleared when item doesn't exist after loading

-

Expected: Focus should be cleared when data is loaded but item doesn't exist

-

-
- -
-

Test 3: Todo/Idea focus always preserved

-

Expected: Focus on todos/ideas should always be preserved (no loading state)

-

-
- - - - diff --git a/test_focus_logic.js b/test_focus_logic.js deleted file mode 100644 index 7a058e5..0000000 --- a/test_focus_logic.js +++ /dev/null @@ -1,163 +0,0 @@ - -#!/usr/bin/env node - -// Simple test to verify the focus service logic -console.log('Testing Focus Service Logic Fix'); -console.log('================================'); - -class MockFocusService { - constructor() { - this.focusTarget = null; - this.ticketsLoading = false; - this.pullRequestsLoading = false; - this.tickets = []; - this.pullRequests = []; - this.todos = []; - this.ideas = []; - } - - setFocus(target) { - this.focusTarget = target; - console.log(`✓ Focus set to: ${JSON.stringify(target)}`); - } - - clearFocus() { - this.focusTarget = null; - console.log('✓ Focus cleared'); - } - - focusedItem() { - if (!this.focusTarget) return null; - return this._resolve(this.focusTarget); - } - - checkFocusValidity() { - console.log(` Current focus: ${JSON.stringify(this.focusTarget)}`); - console.log(` Focused item: ${this.focusedItem() ? 'found' : 'not found'}`); - console.log(` Tickets loading: ${this.ticketsLoading}`); - console.log(` PRs loading: ${this.pullRequestsLoading}`); - - if (this.focusTarget && !this.focusedItem()) { - const dataLoaded = this._isDataLoadedForTarget(this.focusTarget); - console.log(` Data loaded for target: ${dataLoaded}`); - - if (dataLoaded) { - this.clearFocus(); - return 'Focus cleared - data loaded but item not found'; - } else { - return 'Focus preserved - data still loading'; - } - } - return 'Focus valid'; - } - - _resolve(target) { - switch (target.type) { - case 'ticket': - return this.tickets.find(t => t.id === target.id) || null; - case 'pr': - return this.pullRequests.find(p => p.id === target.id) || null; - case 'todo': - return this.todos.find(t => t.id === target.id) || null; - case 'idea': - return this.ideas.find(i => i.id === target.id) || null; - } - return null; - } - - _isDataLoadedForTarget(target) { - switch (target.type) { - case 'ticket': - return !this.ticketsLoading; - case 'pr': - return !this.pullRequestsLoading; - case 'todo': - case 'idea': - return true; // Always available - } - return true; - } -} - -function runTest(testName, testFn) { - console.log(`\n🧪 ${testName}`); - console.log('------------------------------'); - try { - testFn(); - console.log('✅ PASS'); - } catch (error) { - console.log(`❌ FAIL: ${error.message}`); - } -} - -function testFocusPreservedDuringLoading() { - const service = new MockFocusService(); - - // Setup: User sets focus on a ticket - service.tickets = [{ id: 'tk-1', type: 'ticket', title: 'Test Ticket' }]; - service.setFocus({ id: 'tk-1', type: 'ticket' }); - - // Simulate page refresh - data not loaded yet - service.tickets = []; - service.ticketsLoading = true; - - const result = service.checkFocusValidity(); - - if (result !== 'Focus preserved - data still loading') { - throw new Error(`Expected "Focus preserved - data still loading", got "${result}"`); - } - - if (service.focusTarget === null) { - throw new Error('Focus should not be null when data is still loading'); - } -} - -function testFocusClearedWhenItemMissing() { - const service = new MockFocusService(); - - // Setup: User sets focus on a ticket - service.tickets = [{ id: 'tk-1', type: 'ticket', title: 'Test Ticket' }]; - service.setFocus({ id: 'tk-1', type: 'ticket' }); - - // Data loads but ticket is gone - service.tickets = []; - service.ticketsLoading = false; - - const result = service.checkFocusValidity(); - - if (result !== 'Focus cleared - data loaded but item not found') { - throw new Error(`Expected "Focus cleared - data loaded but item not found", got "${result}"`); - } - - if (service.focusTarget !== null) { - throw new Error('Focus should be null when data is loaded but item doesn\'t exist'); - } -} - -function testTodoFocusAlwaysPreserved() { - const service = new MockFocusService(); - - // Setup: User sets focus on a todo - service.todos = [{ id: 'td-1', type: 'todo', title: 'Test Todo' }]; - service.setFocus({ id: 'td-1', type: 'todo' }); - - // Todos are cleared (but no loading state for todos) - service.todos = []; - - const result = service.checkFocusValidity(); - - if (result !== 'Focus cleared - data loaded but item not found') { - throw new Error(`Expected "Focus cleared - data loaded but item not found", got "${result}"`); - } - - if (service.focusTarget !== null) { - throw new Error('Focus should be null when todo doesn\'t exist'); - } -} - -// Run all tests -runTest('Focus preserved during data loading', testFocusPreservedDuringLoading); -runTest('Focus cleared when item missing after loading', testFocusClearedWhenItemMissing); -runTest('Todo focus cleared when item missing', testTodoFocusAlwaysPreserved); - -console.log('\n🎉 All tests completed!');