Skip to content

feat(uve): move default device controls to browser toolbar (#35417)#35419

Merged
zJaaal merged 7 commits intomainfrom
35417-uve-move-device-selector-to-browser-toolbar-and-update-zoom-reset-control
Apr 22, 2026
Merged

feat(uve): move default device controls to browser toolbar (#35417)#35419
zJaaal merged 7 commits intomainfrom
35417-uve-move-device-selector-to-browser-toolbar-and-update-zoom-reset-control

Conversation

@zJaaal
Copy link
Copy Markdown
Member

@zJaaal zJaaal commented Apr 22, 2026

Summary

  • Extracts default device buttons (desktop/tablet/mobile) + orientation toggle into a new dot-uve-device-controls pill component placed in the editor's browser toolbar
  • Strips dot-uve-device-selector down to the "more" button only (custom devices, social media, search engines)
  • Changes zoom reset icon from pi-expand to pi-undo
  • Unifies the device selector change pipeline: toolbar now emits a deviceSelectorChange output, and the editor is the single point that writes to the store (removes the duplicate handleDeviceSelectorChange + $deviceSelectorState from both components)
  • Replaces [class.active] bindings (broken after Tailwind migration) with PrimeNG PT API + bg-primary-50! applied directly to the inner button element
  • Increases pill horizontal/vertical padding (px-3 py-1) and URL bar pill padding for better visual proportions
  • Fixes copy-mode button alignment (justify-start!) in quick-edit panel

Test plan

  • All portlets-edit-ema-portlet tests pass (63 suites, 1325 tests)
  • New dot-uve-device-controls.component.spec.ts covers device selection, orientation toggle, disabled state, and active state computed signals
  • Toolbar spec updated: handleDeviceSelectorChange store-call tests replaced with deviceSelectorChange output emission tests
  • Verify in browser: device controls pill appears in browser toolbar in Preview/Live mode
  • Verify in browser: active device button shows bg-primary-50 background via PT API
  • Verify in browser: orientation button disabled when default device or social media is active
  • Verify zoom reset button shows pi-undo icon

🤖 Generated with Claude Code

This PR fixes: #35417

This PR fixes: #35417

zJaaal and others added 2 commits April 22, 2026 12:24
- Extract default device buttons + orientation into new dot-uve-device-controls pill component
- Strip dot-uve-device-selector down to the "more" button only (custom devices, social media, search engines)
- Move device controls into the editor's browser toolbar alongside URL bar and zoom controls
- Change zoom reset icon from pi-expand to pi-undo
- Unify device selector change handling: toolbar emits output, editor is the single store writer
- Replace [class.active] bindings with PrimeNG PT API + Tailwind bg-primary-50 for active state
- Increase pill padding (px-3 py-1) and URL bar pill vertical padding for better proportions
- Fix copy-mode buttons alignment with justify-start!

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add spec for DotUveDeviceControlsComponent covering device selection,
  orientation toggle, disabled states, and active device computed
- Use it.each for per-device button rendering assertions
- Update toolbar spec: replace handleDeviceSelectorChange store-call tests
  with deviceSelectorChange output emission tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 22, 2026

Claude finished @zJaaal's task in 4m 18s —— View job


PR Review

  • Gather context
  • Read changed files
  • Analyze for issues
  • Post review feedback

Clean refactor overall. Three things worth a second look:


1. $isMoreButtonActive bug — social media not considered

dot-uve-device-selector.component.ts:93

readonly $isMoreButtonActive = computed(() => !this.$state().device?._isDefault);

When a social media option is selected from the "more" menu and the device is still DEFAULT_DEVICE (which has _isDefault: true), this returns false — the "more" button stays unhighlighted even though the user selected something from within it. The $moreButtonLabel already accounts for social media (customDevice?.name || this.$state().socialMedia), but the active state does not. The spec also has no test case for socialMedia !== null with a default device.

Fix:

readonly $isMoreButtonActive = computed(
    () => !this.$state().device?._isDefault || this.$state().socialMedia !== null
);

Fix this →


2. dot-uve-device-controls always visible in edit mode — no guard

edit-ema-editor.component.html:108-111

The device controls pill is always rendered in the browser toolbar. The PR test plan says it should appear in Preview/Live mode. The old dot-uve-device-selector in the top toolbar already guards with $showDeviceSelector(), which returns false in edit mode. The browser toolbar controls lack any such guard.

In edit mode users see a device selector pill they can interact with, even though the top toolbar's dot-uve-device-selector is hidden. If both are intentionally always-visible, the test plan wording is misleading. If it should only show in non-edit mode, it needs an @if:

@if (uveStore.viewMode() !== UVE_MODE.EDIT) {
    <dot-uve-device-controls ... />
}

3. Cross-boundary import for shared types

dot-uve-device-controls.component.ts:12-14

import {
    DeviceSelectorChange,
    DeviceSelectorState
} from '../dot-uve-toolbar/components/dot-uve-device-selector/dot-uve-device-selector.models';

dot-uve-device-controls is a sibling of dot-uve-toolbar, but it reaches into the toolbar's internal component subtree to get shared types. These types are now used by the editor, the toolbar, and the new device-controls component — they've outgrown being owned by dot-uve-device-selector. Moving them to shared/models (or a shared/device-selector.models.ts) would remove this layering smell. Not a runtime issue, but will cause confusion when the toolbar is refactored.


Minor

  • dot-uve-zoom-controls.component.ts:9 has standalone: true explicitly set. Redundant in Angular 19+ (all components are standalone by default). Harmless, just noise.
  • The device controls spec calls component methods directly (onDeviceSelect, onOrientationChange) rather than clicking rendered buttons. The PT-based bg-primary-50! active state is never asserted in tests. If the DOM wiring breaks (e.g., (click) binding removed from template), tests still pass.

zJaaal and others added 4 commits April 22, 2026 13:07
- defaultDevices: was used for default device buttons which moved to dot-uve-device-controls
- $actions and $workflowLoding (typo): duplicate of $workflowIsLoading, never referenced in template

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ntrols

Avoids the transitive dependency through dot-uve-device-selector.component

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ype re-export shim

- data-testId → data-testid (lowercase) on device and orientation buttons
- track $index → track device.inode (inodes are unique stable constants)
- Remove duplicate data-testid from decorative <use> element
- Remove export type re-export shim from dot-uve-device-selector.component
- Update all callers (toolbar ts/spec, device-selector spec) to import
  DeviceSelectorChange/DeviceSelectorState directly from .models file

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@zJaaal zJaaal enabled auto-merge April 22, 2026 17:05
@zJaaal zJaaal added this pull request to the merge queue Apr 22, 2026
Merged via the queue into main with commit 1d65a93 Apr 22, 2026
28 checks passed
@zJaaal zJaaal deleted the 35417-uve-move-device-selector-to-browser-toolbar-and-update-zoom-reset-control branch April 22, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[UVE] Move device selector to browser toolbar and update zoom reset control

2 participants