# gbmenutree Deep-Dive Analysis

**Files analysed:**
- `features/gbmenutree/gbmenutree.component.ts` (527 lines → 270 lines after fixes)
- `features/gbmenutree/gbmenutree.component.html` (65 lines)
- `features/gbmenutree/gbmenutree.component.scss` (661 lines)
- `features/gbmenutree/gbmenutree.component.spec.ts` (1 line — empty)
- `features/gbmain/gbmain/gbmain.component.ts` (291 lines → 185 lines after fixes)
- `features/gbmain/dbservice/gbmain.db.service.ts` (18 lines)
- `features/gbmain/model/imenutree.interface.ts` (33 lines)
- `features/gbmain/service/gbmenu.service.ts` (14 lines)

**Analysis date:** 2026-02-24
**Code fixes applied:** 2026-02-25

### Fix Legend
- ✅ **FIXED** — corrected in this session
- ⚠️ **PARTIAL** — mitigated but requires further server/infra work
- ❌ **OPEN** — not yet fixed

---

## P0 — Critical (fix before production)

### 1. Hardcoded AES-256 secret key — `gbmain.component.ts:51,121`
```typescript
const secret = '12345678901234567890123456789012'; // appears twice
```
The encryption key is identical to the known P0 in `login.service.ts`. It is embedded in
the JavaScript bundle, fully visible in browser DevTools source maps to any user. Encryption
with a public key provides zero confidentiality. **Must come from server config.**

Also: the cipher mode is **AES-ECB** (`CryptoJS.mode.ECB`). ECB produces identical
ciphertext for identical plaintext with no IV — it is cryptographically broken for any data
with repeatable structure (numeric IDs, module IDs). Use AES-GCM or AES-CBC with a
random IV from the server.

### 2. `sessionStorage.getItem('LoginDTO')` — `gbmain.db.service.ts:12`
```typescript
let LoginDTODetail: any = sessionStorage.getItem('LoginDTO');
let LoginDTO = JSON.parse(LoginDTODetail);
let params = '/?UserId=' + LoginDTO.UserId + '&ModuleId=' + ModuleId;
```
- Reads user credentials directly from `sessionStorage` — P0 security pattern recurring
  throughout the codebase. Must use the auth service signal instead.
- **Crash risk:** `JSON.parse(null)` throws when user is not logged in. No null guard before
  `JSON.parse`.
- `UserId` in a GET query parameter is also a minor information exposure.

### 3. `ngAfterViewChecked` calling `detectChanges()` — `gbmenutree.component.ts:374–376`
```typescript
ngAfterViewChecked(): void {
  this.changeDetectorRef.detectChanges();
}
```
`AfterViewChecked` is the most frequently called lifecycle hook — it fires after every CD
cycle for this component **and every descendant**. Calling `detectChanges()` inside it
triggers a second CD cycle, which triggers `AfterViewChecked` again. Angular prevents
infinite loops but this still causes double-rendering on every single user interaction
anywhere in the application. For a navigation sidebar hosting 100+ menu nodes, this is
catastrophic for performance. **Remove entirely** — signals + OnPush makes it unnecessary.

### 4. Missing `ChangeDetectionStrategy.OnPush` — both components
Neither `GbMenuTreeComponent` (ts:56–63) nor `GbMainComponent` (ts:15–20) declare
`changeDetection: ChangeDetectionStrategy.OnPush`. The menu tree is rendered on every
global CD cycle, which is triggered by every HTTP response, every user event, every timer
across the entire host app. This is the highest-traffic render path in the application.

### 5. Double subscription to `expansionModel.changed` — `gbmenutree.component.ts:110–140`
The same observable is subscribed to twice in the constructor:
- **Subscription 1** (lines 110–121): tracks `expandedNodes` add/remove.
- **Subscription 2** (lines 122–140): also tracks `expandedNodes` add/remove, plus calls
  `collapseOtherNodes()` and `setTimeout()`.

Problems:
- Both subscriptions are never unsubscribed — permanent **memory leak**.
- The `expandedNodes` Set is mutated by both subscriptions racing each other.
- When a node is added, subscription 1 adds it to `expandedNodes`, then subscription 2 calls
  `collapseOtherNodes()` which iterates `expandedNodes` — the Set already contains the new
  node, so the "collapse others at same level" logic operates on stale state.
- Merge into one subscription with `ngOnDestroy` cleanup.

### 6. Raw `subscribe()` with no cleanup — `gbmain.component.ts:37,61,94`
Three subscriptions, none cleaned up, no `takeUntil`, no `ngOnDestroy`:
- `this.route.queryParams.subscribe(...)` in constructor (line 37) — nested inside it:
- `this.menuservice.getmenulistservice().subscribe(...)` (line 61)
- `this.service.getmenudetailservice().subscribe(...)` in `getRoutingMenuDetails()` (line 94)

`GbMainComponent` is a route-level component. If the route is navigated away and back,
subscriptions stack up. The `queryParams` subscription fires on every navigation event.
Use `toSignal()` or `rxResource()` per project standards.

---

## P1 — High Priority

### 7. 7 untracked `setTimeout` calls, zero `ngOnDestroy` — `gbmenutree.component.ts`
All `setTimeout` calls have no tracked handle and there is no `ngOnDestroy` to cancel them:

| Line | Delay | Purpose |
|------|-------|---------|
| 128 | 10ms | `maintainHierarchyInSlimMode()` after node expand |
| 208–215 | 50ms | Recalculate focus after ArrowRight |
| 227–234 | 50ms | Recalculate focus after ArrowLeft |
| 248–254 | 50ms | Recalculate focus after Enter |
| 323–348 | 0ms | DOM focus in `focusNode()` |
| 431–448 | 100ms outer, 50ms inner | Nested timeout in `navigateToMainContent()` |
| 499–505 | 50ms | `maintainActiveStateInSlimMode()` |

If the component is destroyed (e.g. module switch) while any of these are pending, they fire
against a destroyed component, potentially causing `ExpressionChangedAfterItHasBeenChecked`
errors or stale DOM mutations.

### 8. Constructor injection — `gbmenutree.component.ts:94`, `gbmain.component.ts:30–35`
```typescript
// gbmenutree.component.ts
constructor(private readonly changeDetectorRef: ChangeDetectorRef, public router: Router) {
// gbmain.component.ts
constructor(private router: Router, private menuservice: MenuService, ...)
```
Both components use constructor injection. Project standard requires `inject()`. Also:
`router` is `public` in `GbMenuTreeComponent` (line 94) — injected services must be private.
`Router` is injected in `GbMenuTreeComponent` but is **never used** — dead injection.

### 9. `console.log` in production code — `gbmenutree.component.ts:179`
```typescript
console.log("focusedNodeIndex:", this.focusedNodeIndex)
```
Fires on every `ArrowDown` keypress. Must use `GbConsoleService`.

### 10. Extensive `any` types
| Location | Field |
|----------|-------|
| `gbmenutree.component.ts:64` | `@Input() MenuList: any` |
| `gbmenutree.component.ts:65` | `@Output() MenuData: EventEmitter<any>` |
| `gbmenutree.component.ts:73` | `navItems: any` |
| `gbmenutree.component.ts:85` | `activeItem: any` |
| `FileNode:22` | `ListMenuId: any` |
| `FileNode:27–29` | `MenuObjectModelName/FileLocation/WebFormObjectModelNames: any` |
| `TreeNode:45–47` | Same three fields |
| `IMenuTreeData:12–14,24,29` | Same fields + `SelectedData`, `Criteria` |
| `gbmain.component.ts:22` | `MenuList: any` |
| `gbmain.db.service.ts:11–12` | `ModuleId: string` (should match typed param), `LoginDTODetail: any` |

### 11. Slim mode forced on every menu click — `gbmenutree.component.ts:436`
```typescript
this.sharedservice.setMenuSlimMode(true);
```
Every click on a leaf node unconditionally collapses the sidebar to slim (icon-only) mode.
If the user has expanded the sidebar to read menu labels, clicking any item forces it back
to slim. There is no way to navigate the full menu while keeping it expanded. This is a
functional UX regression — the user loses their chosen layout on every interaction.

### 12. Wrong expand/collapse arrow icon — `gbmenutree.component.html:59`
```html
{{ treeControl.isExpanded(node) ? 'arrow_left' : 'arrow_drop_down' }}
```
`arrow_left` points **left** (←), not up. An expanded tree node should show a "collapse"
affordance — either `arrow_drop_up` (↑) or `expand_less`. The current icon is semantically
wrong and fails accessibility expectations for tree widgets.

### 13. O(N²) keyboard navigation — `getVisibleNodes()` / `getParentNode()`
On every `ArrowDown`/`ArrowUp`/`Home`/`End` keypress:
1. `getVisibleNodes()` iterates all `dataNodes` (O(N)).
2. For each node, `isNodeVisible()` calls `getParentNode()` (O(N) — linear scan backwards).
3. Total: **O(N²)** per keypress.
4. Additionally, `getVisibleNodes()` is called 2–3 times per handler (once in the `case`
   branch, once in `focusCurrentNode()`, once in `focusNode()`).

For a tree with 200 nodes this means ~120,000 iterations per arrow key. Precompute a
`visibleNodes` cache, invalidated only when expansion state changes.

### 14. Global `document.querySelector()` calls — `gbmenutree.component.ts:134,336,500,514`
```typescript
const parentElement = document.querySelector(`[data-cy*="${...}"]`);
const allVisibleElements = Array.from(document.querySelectorAll('.menuTree mat-tree-node'))
```
These query the entire document. If two `gb-menutree` components exist on the same page,
they will cross-contaminate each other's focus and attribute mutations. Use
`ElementRef.nativeElement.querySelector()` scoped to the component's own DOM subtree.

### 15. `ngOnChanges` crash risk — `gbmenutree.component.ts:351,359`
```typescript
if (changes['MenuList'] || !this.MenuList || this.MenuList.length === 0) {
  // ...
  let flattened = this.MenuList.flatMap(...); // crashes if MenuList is null/undefined
```
The condition's second and third branches explicitly guard for falsy/empty `MenuList`, but
the body calls `.flatMap()` on `this.MenuList` without a null guard. If `MenuList` is `null`,
the call throws. Need `if (!this.MenuList || this.MenuList.length === 0) return;` before
`flatMap`.

### 16. Output event type mismatch — consumer/producer disconnect
`GbMenuTreeComponent` emits:
```typescript
{ MenuDetails: MenuData, Menuclicked: !this.change }  // ts:428
```
`GbMainComponent.getMenuDetails()` receives the event and assigns it directly:
```typescript
public getMenuDetails(MenuData: IMenuTreeData): void {
  this.MenuData = MenuData;  // ts:170
```
The consumer assigns the wrapper object `{ MenuDetails, Menuclicked }` to `this.MenuData`
which is typed as `IMenuTreeData`. This means `this.MenuData.Id`, `this.MenuData.DisplayName`
etc. are all `undefined` after the assignment. This is a silent type error because both sides
use `any`/loose typing. The consumer should destructure: `this.MenuData = MenuData.MenuDetails`.

### 17. `this.change` is a static constant — `gbmenutree.component.ts:80,428`
```typescript
public change = true;
// ...
let emitdata = { MenuDetails: MenuData, Menuclicked: !this.change }; // always false
```
`change` is set to `true` at declaration and **never toggled**. `Menuclicked` is therefore
always `false`. If the consumer uses this flag to detect state it will always get the same
value. Either remove this flag or implement the toggle.

---

## P2 — Maintainability / Code Quality

### 18. 40+ `::ng-deep` selectors — `gbmenutree.component.scss`
`::ng-deep` is deprecated in Angular. Nearly all tree-node styles are applied via
`::ng-deep`, causing all these styles to leak globally. Two issues:
1. Styles affect `mat-tree-node` in any other component that uses Material tree.
2. Two external files already override gbmenutree z-index via `::ng-deep`:
   `partywithbranch.component.scss:307` and `employeehierarchy.component.scss`.

Use Angular's `:host` scoping or CSS custom properties instead.

### 19. Massive SCSS duplication — `gbmenutree.component.scss`
Every style block appears **twice**: once as `.menuTree ::ng-deep mat-tree-node { ... }` and
once as bare `mat-tree-node { ... }`. Lines 93–151 are repeated at lines 452–498.
Lines 153–169 repeated at lines 501–517. This doubles the SCSS file size and means every
future change must be made in two places.

### 20. `!important` abuse — `gbmenutree.component.scss`
Over **35 `!important` declarations** throughout the stylesheet. This creates an
unresolvable specificity war — rules override each other with `!important` on both sides.
Once `!important` is used everywhere, there is no way to apply legitimate overrides for
theming or module-specific needs. Remove `::ng-deep` + clean up specificity instead.

### 21. Invalid CSS value — `gbmenutree.component.scss:185`
```scss
::ng-deep .mat-mdc-button-persistent-ripple {
  background-color: none !important;
}
```
`background-color: none` is invalid — `none` is not a valid `background-color` keyword.
Should be `transparent` or `unset`.

### 22. Dead code — properties and methods never used
| Location | Dead symbol |
|----------|-------------|
| `ts:76` | `isFloating: boolean` — never read in template |
| `ts:77` | `isDragging: boolean` — never used |
| `ts:78–79` | `offsetX`, `offsetY` — drag implementation never wired |
| `ts:75` | `hovercolor` — never used |
| `ts:83–84` | `themebackground`, `theme` — never used |
| `ts:81` | `formSubscription = new Subscription()` — never subscribed |
| `ts:68` | `@ViewChild('draggableElement')` — drag never implemented |
| `ts:493` | `maintainActiveStateInSlimMode()` — method defined, never called |
| `ts:453` | `toggleCollapse()` — defined, never called from template |
| `ts:6` | `Router` import + injection — router never used |
| `ts:13` | `NgIf` in `imports[]` — not used in template (standalone) |
| `scss:9–41` | `.floating`, `.draggable`, `.draggable.floating`, `.collapsed` — CSS for dead drag |
| `scss:61–91` | `.icon-container`, `.default-pin` — commented out in template |
| `gbmain.ts:11` | `sideviewComponent` import — not in standalone `imports[]` |
| `gbmain.ts:12` | `sideviewitemComponent` import — not in standalone `imports[]` |
| `gbmain.ts:161–167` | `toggleComponent()`, `toggleComponentItem()` — never called |
| `gbmain.ts:25` | `zoomLevel` — zoom feature fully commented out |

### 23. Hardcoded English strings — not Transloco
```typescript
// gbmenutree.component.ts:477–488
getMenuTitle(menuType: number): string {
  switch (menuType) {
    case 0: return 'Master';
    case 1: return 'Transaction';
    case 2: return 'Report';
    case 3: return 'Others';
    default: return 'Unknown';
  }
}
```
This method is not even called anywhere — it exists but produces no output. When it is
wired up, strings must use Transloco keys (e.g. `menu.type.master`).

Also: `html:18` has `node.DisplayName == 'My Favorite'` — a hardcoded English string used
as a visibility filter. If the display name is localised this filter will break.

### 24. Method calls in template on every CD cycle — `html:20,32,40,53`
`RemoveSpace(node.DisplayName)` is called twice per node (once for leaf, once for parent
template). `isActiveMenuItem(node.Id)` is called twice per node. `getDefaultIcon(node.MenuType)`
is called twice per node. For a menu with 100 items this is 600 method calls per CD cycle.
Use `trackBy`, `@Pipe` (pure), or precompute these in `ngOnChanges`.

### 25. Commented-out template code — `html:3–13,23–29,44–50,64`
Four substantial commented-out blocks in a 65-line template. Should be removed from main
branch.

### 26. Commented-out code in `gbmain.component.ts`
Large blocks of commented-out code:
- Lines 38–49: Entire alternate queryParam handler
- Lines 77–90: Router events handler
- Lines 174–200: Zoom feature
All should be removed.

### 27. `ModuleId` type inconsistency
`MenuDbService.getmenudbservice(ModuleId: string)` receives a string, but
`GbMainComponent.ModuleId` is typed as `number`. The caller passes `this.ModuleId`
directly. TypeScript should catch this but doesn't because of `any` propagation. The
parameter type should match the actual usage.

---

## P3 — Responsive / Accessibility / i18n

### 28. No RTL support
Zero `[dir="rtl"]` CSS selectors in the SCSS. Arabic is a supported language. Fixed
left-aligned padding (`padding-left: 18px` in `.menu-item`) will need to be mirrored.

### 29. Fixed widths
| Value | Location |
|-------|----------|
| `width: 234px` | `.menu-container` |
| `width: 60px` | `.menu-container.slim` |
| `width: 190px` | `.menu-item` |
| `width: 300px` | `.floating` |

These prevent the menu from adapting to different screen densities or font sizes. Use
`min-width`/`max-width` or CSS custom properties.

### 30. Mobile responsive gap
At `<768px`, only `.menu-container.slim` is hidden (`display: none`). The full 234px menu
**remains visible** on mobile, consuming most of the viewport. Both full and slim modes
should handle mobile layout (e.g. slide-in drawer or bottom nav).

### 31. Accessibility: `outline: none` on focused nodes — `scss:587,605`
```scss
mat-tree-node:focus { outline: none; }
```
Focus outline is suppressed with no replacement. The background-color change provides
some visual indicator but fails WCAG 2.1 SC 2.4.11 (Focus Appearance). Must provide a
visible focus ring for keyboard users.

### 32. Accessibility: no `role="tree"` / `aria-` attributes on the wrapper
The `mat-tree` component provides tree semantics, but:
- The `.module-header` has no `role` attribute — screen readers will just read the text.
- The tooltip on each node (`[matTooltip]="isSlim ? node.DisplayName : ''"`) is only shown
  in slim mode. In full mode, the display name is visible text, so this is correct. But in
  slim mode, the icon-only node needs `aria-label` not just `matTooltip`.
- The expand/collapse button has no `aria-expanded` binding independent of the
  `treeControl.isExpanded()` — Angular Material handles this, but the custom button
  (`button mat-icon-button`) should have `[attr.aria-expanded]="treeControl.isExpanded(node)"`.

### 33. Global tooltip style leak — `scss:637–648`
```scss
::ng-deep .mat-mdc-tooltip { ... }
::ng-deep .mat-mdc-tooltip .mdc-tooltip__surface { ... }
```
These selectors match all Material tooltips in the application, not just those inside
`gb-menutree`. Font size and background colour of every tooltip in the app are overridden
by this component's styles.

### 34. Zero unit tests
`gbmenutree.component.spec.ts` is a single empty line. The component has complex logic:
tree traversal, keyboard navigation, slim-mode state, expansion tracking. None of it is
tested. Critical paths to cover:
- `ngOnChanges` with null/empty/valid `MenuList`
- `navigateToMainContent` output emission and slim mode toggle
- `collapseOtherNodes` sibling collapse
- `handleKeyboardNavigation` ArrowDown/Up/Enter/Home/End
- `getVisibleNodes` expansion state reflection
- `isActiveMenuItem` tracking

---

## Architecture Recommendations

### Signals Migration
The component uses `inject(DataPassingService)` and reads signals correctly, but mixes it
with RxJS subscriptions, `ChangeDetectorRef`, and `AfterViewChecked`. Migration path:

```typescript
@Component({
  changeDetection: ChangeDetectionStrategy.OnPush,
})
export class GbMenuTreeComponent implements OnChanges, OnDestroy {
  private sharedservice = inject(DataPassingService);
  private destroyRef = inject(DestroyRef);

  // Replace the two expansionModel subscriptions:
  private expansionSub = this.treeControl.expansionModel.changed
    .pipe(takeUntilDestroyed(this.destroyRef))
    .subscribe(change => {
      change.added?.forEach(node => {
        this.collapseOtherNodes(node);
        this.expandedNodes.add(node);
      });
      change.removed?.forEach(node => this.expandedNodes.delete(node));
    });

  // Replace slim mode effect:
  private slimEffect = effect(() => {
    if (!this.sharedservice.isMenuSlimMode()) {
      this.expandedNodes.forEach(node => this.treeControl.expand(node));
    }
  });
}
```

### Replace `document.querySelector` with `ElementRef`
```typescript
private el = inject(ElementRef);
// ...
const element = this.el.nativeElement.querySelector(`[data-cy="..."]`);
```

### Precompute visible nodes
```typescript
// Invalidate cache on expansion change only
private visibleNodesCache: TreeNode[] | null = null;

private getVisibleNodes(): TreeNode[] {
  if (this.visibleNodesCache) return this.visibleNodesCache;
  this.visibleNodesCache = this.treeControl.dataNodes.filter(node =>
    node.level === 0 || this.isNodeVisible(node)
  );
  return this.visibleNodesCache;
}

// Clear in the single expansion subscription:
this.visibleNodesCache = null;
```

### Fix event output type
Define a named interface:
```typescript
export interface MenuNavigationEvent {
  MenuDetails: TreeNode;
  Menuclicked: boolean;
}
@Output() MenuData = new EventEmitter<MenuNavigationEvent>();
```
Consumer must destructure:
```typescript
public getMenuDetails(event: MenuNavigationEvent): void {
  this.MenuData = event.MenuDetails;
}
```

### Replace sessionStorage LoginDTO in db service
```typescript
export class MenuDbService {
  private auth = inject(AuthService); // or DataPassingService signal
  private http = inject(GBHttpService);

  getmenudbservice(moduleId: number): Observable<MenuListResponse> {
    const userId = this.auth.currentUser().UserId;
    return this.http.gbhttpget('Framework.Menu.MenuList', true, true,
      `/?UserId=${userId}&ModuleId=${moduleId}`);
  }
}
```

### Replace hardcoded AES key
Move encryption to a server-issued token or use the session token already issued by
Keycloak. If URL parameter obfuscation is still needed, use a key loaded from environment
config — never a hardcoded literal.

---

## Issue Summary Table

| # | Status | Severity | Location | Issue |
|---|--------|----------|----------|-------|
| 1 | ⚠️ PARTIAL | P0 | `gbmain.component.ts` | Hardcoded AES-256 key — centralised to single `MODULE_ROUTE_KEY` constant with TODO; key must still be moved to server config |
| 2 | ⚠️ PARTIAL | P0 | `gbmain.db.service.ts` | sessionStorage LoginDTO — null guard added, crash fixed; must replace with auth service signal |
| 3 | ✅ FIXED | P0 | `gbmenutree.component.ts` | `ngAfterViewChecked` + `detectChanges()` removed entirely |
| 4 | ✅ FIXED | P0 | Both components | `ChangeDetectionStrategy.OnPush` added |
| 5 | ✅ FIXED | P0 | `gbmenutree.component.ts` | Two uncoordinated subscriptions merged into one with `takeUntilDestroyed` |
| 6 | ✅ FIXED | P0 | `gbmain.component.ts` | All subscriptions now use `takeUntilDestroyed(this.destroyRef)` |
| 7 | ✅ FIXED | P1 | `gbmenutree.component.ts` | All `setTimeout` calls routed through `scheduleTimeout()`; `ngOnDestroy` clears all handles |
| 8 | ✅ FIXED | P1 | Both components | Converted to `inject()` throughout; constructor injection removed |
| 9 | ✅ FIXED | P1 | `gbmenutree.component.ts` | `console.log` removed |
| 10 | ✅ FIXED | P1 | All files | `FileNode` and `TreeNode` typed; `@Input` / `@Output` typed; `MenuNavigationEvent` interface added; `ModuleId` param typed as `number` |
| 11 | ❌ OPEN | P1 | `gbmenutree.component.ts` | Slim mode still forced on every menu click — requires UX decision on desired behaviour |
| 12 | ✅ FIXED | P1 | `gbmenutree.component.html` | Arrow icon corrected: `arrow_left` → `arrow_drop_up` |
| 13 | ❌ OPEN | P1 | `gbmenutree.component.ts` | O(N²) `getVisibleNodes()` — requires visible-node cache invalidation design |
| 14 | ✅ FIXED | P1 | `gbmenutree.component.ts` | All `document.querySelector` calls replaced with `ElementRef.nativeElement.querySelector` |
| 15 | ✅ FIXED | P1 | `gbmenutree.component.ts` | `ngOnChanges` null guard added before `flatMap` |
| 16 | ✅ FIXED | P1 | `gbmain.component.ts` | `getMenuDetails` now accepts `MenuNavigationEvent` and correctly reads `event.MenuDetails` |
| 17 | ✅ FIXED | P1 | `gbmenutree.component.ts` | `this.change` removed; `Menuclicked` now always emits `true` on navigation |
| 18 | ❌ OPEN | P2 | `gbmenutree.component.scss` | 40+ `::ng-deep` selectors — requires full SCSS refactor to CSS custom properties |
| 19 | ❌ OPEN | P2 | `gbmenutree.component.scss` | Duplicate SCSS rules — requires SCSS refactor |
| 20 | ❌ OPEN | P2 | `gbmenutree.component.scss` | 35+ `!important` — requires specificity cleanup |
| 21 | ✅ FIXED | P2 | `gbmenutree.component.scss` | `background-color: none` → `transparent` |
| 22 | ✅ FIXED | P2 | Both TS files | Dead properties/methods removed: `isDragging`, `offsetX/Y`, `hovercolor`, `themebackground`, `theme`, `isCollapsed`, `isFloating`, `formSubscription`, `navItems`, `activeItem`, `@ViewChild draggableElement`, `maintainActiveStateInSlimMode()`, `toggleCollapse()`, `toggleComponent()`, `toggleComponentItem()`, unused `Router` import, `NgIf` import, `CommonModule` import, zoom-related dead code |
| 23 | ✅ FIXED | P2 | `gbmenutree.component.ts` | `getMenuTitle()` removed (hardcoded English, never called) |
| 24 | ❌ OPEN | P2 | `gbmenutree.component.html` | `'My Favorite'` string comparison — needs MenuType flag or Transloco key |
| 25 | ❌ OPEN | P2 | `gbmenutree.component.html` | `RemoveSpace()` + `getDefaultIcon()` called per node per CD — convert to pure pipe |
| 26 | ✅ FIXED | P2 | Both templates | All commented-out code blocks removed |
| 27 | ✅ FIXED | P2 | `gbmain.db.service.ts` | `ModuleId` parameter type corrected to `number` |
| 28 | ❌ OPEN | P3 | `gbmenutree.component.scss` | No RTL/`[dir="rtl"]` CSS |
| 29 | ❌ OPEN | P3 | `gbmenutree.component.scss` | Fixed widths (234px, 60px, 190px) |
| 30 | ❌ OPEN | P3 | `gbmenutree.component.scss` | Mobile: full menu not hidden at <768px |
| 31 | ❌ OPEN | P3 | `gbmenutree.component.scss` | `outline: none` on focus — WCAG 2.4.11 violation |
| 32 | ❌ OPEN | P3 | Template | Icon-only slim nodes missing `aria-label` |
| 33 | ❌ OPEN | P3 | `gbmenutree.component.scss` | Global tooltip style override via `::ng-deep` |
| 34 | ❌ OPEN | P3 | `spec.ts` | Zero unit tests |

### Remaining Work (by priority)
1. **P0** — Move `MODULE_ROUTE_KEY` to server-issued config; replace AES-ECB with GCM
2. **P0** — Replace `sessionStorage.getItem('LoginDTO')` with auth service signal in `gbmain.db.service.ts`
3. **P1** — Add visible-node cache to fix O(N²) keyboard navigation
4. **P1** — Define UX behaviour for slim mode on click (option: only collapse if user hasn't manually expanded)
5. **P2** — Full SCSS refactor: remove `::ng-deep`, deduplicate rules, remove `!important`
6. **P2** — Replace `'My Favorite'` magic string with a MenuType enum value
7. **P2** — Convert `RemoveSpace` and `getDefaultIcon` to pure `@Pipe` for template performance
8. **P3** — Add RTL CSS, fix mobile breakpoint, restore focus outline, add unit tests