Skip to content

MWPW-191570: Studio pagination, search refill, and translation dialog UX#742

Merged
Roycethan merged 47 commits intomainfrom
MWPW-191570
Apr 17, 2026
Merged

MWPW-191570: Studio pagination, search refill, and translation dialog UX#742
Roycethan merged 47 commits intomainfrom
MWPW-191570

Conversation

@Axelcureno
Copy link
Copy Markdown
Member

@Axelcureno Axelcureno commented Apr 3, 2026

Overview

This PR improves Studio pagination, search behavior, and the translation items dialog UX:

  1. Skeleton shimmer loading for translations and placeholders admin tables
  2. Pagination plumbing fixes — sentinel cascade guards, RAF re-observe, eager-load cap, re-entrancy guard for processCardsData
  3. Search refill loop — auto-fetches additional cursor pages when a narrow filter leaves fewer than 25 visible items, eliminating the "one result, then loading, then restart" UX
  4. Translation dialog UX — close (X) icon + backdrop dismiss, global search bar above tabs with 300ms debounce, reduced whitespace

Resolves

https://jira.corp.adobe.com/browse/MWPW-191570

Key Changes

Search Refill Loop (mas-repository.js)

  • MIN_FILTERED_PAGE_RESULTS = 25 constant with JSDoc
  • #refillBelowThreshold() — mirrors #eagerLoadAllPznPages pattern: fire-and-forget from the non-personalization branch, identity-based abort, AbortError catch
  • Wired into searchFragments else branch — only runs when personalization is OFF and cursor has more pages
  • Renames MAX_EAGER_PAGES to MAX_EAGER_PZN_PAGES with JSDoc

Translation Dialog UX (mas-items-selector.js, mas-search-and-filters.js, mas-translation-editor.js)

  • dismissable attribute on sp-dialog-wrapper — adds close icon + backdrop-click-to-close
  • Search bar moved from per-tab to global dialog header next to "Select items" title
  • 300ms debounce on search input (was per-keystroke)
  • Filter reactivity consolidated through willUpdate — removed redundant direct #applyFilters() calls

Translation Re-entrancy (translation-items-loader.js)

  • processCardsData stashes pending payloads instead of silently dropping concurrent invocations
  • DRY table-head template extracted in mas-translation.js

Pagination Foundation (earlier commits)

  • Sentinel cascade guard on non-empty items list
  • RAF-deferred sentinel re-observe to prevent cascade loads
  • MAX_EAGER_PZN_PAGES = 20 cap on eager personalization prefetch
  • Skeleton shimmer for translations and placeholders admin tables

Test URLs

Test Coverage

  • 5 new refill-below-threshold test cases (happy path, early termination, cursor exhaustion, abort, pzn skip)
  • 3 existing pagination tests updated for new refill behavior
  • 25 search-and-filters tests updated for external searchQuery property pattern
  • Translation skeleton test aligned with shimmer UI

QA Checklist

  • C1. Unit tests pass locally (1502 passing, 5 pre-existing failures unrelated)
  • C2. NALA test (check with #fishbags if needed)
  • C3. All checks green
  • C4. Test URLs in PR description
  • C5. Demo ready
  • C6. JIRA ACs validated

Screenshots

(Add before/after screenshots of narrow-filter refill and dialog UX changes)

@aem-code-sync
Copy link
Copy Markdown

aem-code-sync bot commented Apr 3, 2026

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-sync branch
Commits

Axelcureno and others added 7 commits April 3, 2026 14:44
…ort-restart with re-entrancy guard in processCardsData
Fixed 28 failing tests by moving setupCardsInStore/setupCollectionsInStore/setupPlaceholdersInStore calls to occur AFTER element creation instead of BEFORE. The tests now properly follow the pattern:
1. Create element
2. Wait for initial subscription (updateComplete)
3. Setup store data
4. Wait for updates (updateComplete)

This fixes the root cause where store initialization in connectedCallback was overwriting test setup data.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…bles

Replace sp-progress-circle spinners with skeleton shimmer rows that match
table column structure for better UX and loading state visibility.

Changes:
- Create studio/src/common/skeleton-styles.css.js with shared shimmer animation
- Add skeleton rows to translations table (4 columns, 5 rows)
- Add skeleton rows to placeholders table (7 columns, 5 rows)
- Skeleton cells inherit widths from table column CSS classes
- Remove loadingIndicator() from placeholders component
- Fixes animation duplication in mas-fragment-editor.js and merch-card-editor.js

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@Axelcureno
Copy link
Copy Markdown
Member Author

Update: Skeleton Shimmer Loading States

Just pushed an additional commit that adds skeleton shimmer loading states to the Translation Projects and Placeholders tables.

What was added:

  • New module: studio/src/common/skeleton-styles.css.js — Shared shimmer animation CSS
  • Translation table: 5 skeleton rows (4 columns) replacing sp-progress-circle spinner
  • Placeholders table: 5 skeleton rows (7 columns) replacing sp-progress-circle spinner

Benefits:

✅ Better UX — skeleton rows provide visual continuity
✅ Single source of truth for shimmer animation (fixes duplication in mas-fragment-editor.js and merch-card-editor.js)
✅ Skeleton cells inherit column widths from table CSS — automatic layout consistency
✅ Table head remains visible during loading

Design:

  • Separation of concerns: Shared CSS for animation logic; per-component skeleton row templates
  • Reusability: Future components can import skeletonStyles for consistent loading states
  • No dynamic helpers: Simple, locally-defined template functions maintain clarity

All files linted and committed: fd4c6cc50

Ready for testing on the staging URLs!

Change skeleton condition from 'loading && internalPlaceholders.length === 0'
to 'loading' only. The previous condition caused skeleton to disappear and
briefly show "No placeholders found" before new data arrived, due to:

1. Race condition with loadPreviewPlaceholders() clearing loading state
2. internalPlaceholders retaining old data from previous locale

Now:
- Skeleton shows for entire loading duration regardless of data state
- "No placeholders found" only appears after loading completes with no results
- No flash of zero results between skeleton and data appearance

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@Axelcureno
Copy link
Copy Markdown
Member Author

Fix: Placeholders skeleton UX issue

Just pushed a fix for the skeleton shimmer disappearing and briefly showing "No placeholders found" before data loads.

Root Cause

The skeleton condition checked two states:

this.loading && this.internalPlaceholders.length === 0

This caused two issues:

  1. Race condition: loadPreviewPlaceholders() shares Store.placeholders.list.loading and clears it in its own finally block, potentially before loadPlaceholders() data arrives
  2. Stale data: internalPlaceholders retained old data from previous locale, so skeleton never showed on locale switch

The Fix

Simplified to check this.loading alone — skeleton shows for entire loading duration, empty state only appears after loading completes:

${this.loading
    ? Array.from({ length: 5 }, placeholdersSkeletonRow)
    : repeat(this.internalPlaceholders, ...)}
${!this.loading && this.internalPlaceholders.length === 0
    ? html`<p class="no-placeholders-label">No placeholders found</p>`
    : nothing}

Result: Smooth loading experience with no flashing of "zero results"

Commit: dfac0ea3c

@Axelcureno Axelcureno marked this pull request as draft April 5, 2026 20:50
@Axelcureno Axelcureno self-assigned this Apr 9, 2026
…ocessCardsData

Extract duplicated sp-table-head markup into a translationProjectsTableHead
getter. Replace inline style with CSS class for align-right. Fix
processCardsData to stash pending payloads instead of silently dropping
them during concurrent invocations. Update skeleton-row test assertions
to match the shimmer UI shipped in fd4c6cc.
When a cursor page, after #filterStoresByPersonalizationEnabled, has
fewer than MIN_FILTERED_PAGE_RESULTS (25) visible items AND the cursor
is not exhausted, the new #refillBelowThreshold loop fetches additional
cursor pages until the threshold is met or the cursor runs out.

This eliminates the narrow-filter UX where applying a tight tag
combination shows "1 result, loading, restart" instead of a full page.
The loop mirrors #eagerLoadAllPznPages: fire-and-forget, identity-based
abort on filter change, AbortError catch. It only runs in the
non-personalization branch; the pzn path already has its own eager
loader.

Also renames MAX_EAGER_PAGES → MAX_EAGER_PZN_PAGES with JSDoc.
…n tests

Five new test cases for #refillBelowThreshold:
- filtered count below threshold → refill continues to 25+
- all-visible pages → refill still reaches threshold
- cursor exhausted before threshold → terminates with hasMore=false
- extremely narrow filter (20 pages, 1 visible each) → no infinite loop
- personalization ON → refill skipped, eager-pzn runs instead

Three existing pagination tests updated: first-page item count boosted
to MIN_FILTERED_PAGE_RESULTS so the refill loop does not fire, keeping
these tests focused on their original loadNextPage intent.
- selectedCount now sums .length instead of spreading three arrays;
  saves per-render allocation in both mas-items-selector and
  mas-translation-editor (getter called multiple times per render).
- Hoist selectedCount and 'Hide selection'/'Selected items' label
  ternary to locals in mas-items-selector render; removes three
  repeated computations in the toggle pill template.
- Collapse #dispatchConfirmItems and #dispatchCancelItems (identical
  except for the event name) into a single #dispatchDialogEvent(name).
- Document the #itemsConfirmed sticky-flag invariant above
  #restoreItemsSnapshot so a future reader doesn't reintroduce the
  clear-on-first-close bug.
- should keep confirmed selection when sp-dialog-wrapper emits a
  duplicate close event (catches clear-on-first-close regression).
- should ignore sp-opened events bubbling from nested overlay-triggers
  (catches regression where a filter popover open wiped filter state).
- should stop filter checkbox change events from bubbling to
  ancestors (catches regression where sp-tabs @change fired on
  checkbox toggle and clobbered selectedTab).
@Roycethan
Copy link
Copy Markdown
Collaborator

https://jira.corp.adobe.com/browse/MWPW-191570

#4 and #5 are fixed , introduced this issue: #6 : When filter applied on project modal its breaking the view and no results returned: image

#6 filter applied works now
#7 Auto type search doesn't work

The debounced #handleSearchInput previously read e.target.value, but
since the input event bubbles through sp-search's shadow DOM and is
retargeted at the mas-translation-editor host, e.target pointed at
MAS-TRANSLATION-EDITOR (value=undefined) instead of the sp-search.
searchQuery stayed empty until the user pressed Enter (which hit the
submit handler with the same bug, but sp-search handles Enter
internally and sets its own value).

Read e.currentTarget?.value instead (sp-search is the listener host,
so currentTarget is always sp-search). Capture the value synchronously
before handing it to the debounce, since currentTarget is nulled out
after event dispatch.
@Roycethan
Copy link
Copy Markdown
Collaborator

https://jira.corp.adobe.com/browse/MWPW-191570

#4 and #5 are fixed , introduced this issue: #6 : When filter applied on project modal its breaking the view and no results returned: image

#6 filter applied works now #7 Auto type search doesn't work

#7 - fixed

When running from a worktree checkout, detect the branch's AEM port
offset from worktrees/.ports and return http://localhost:<port>.
Falls back to 3000 (main mas/ repo) when not inside a worktree.

Before: developers in a worktree had to remember to use
`LOCAL_TEST_LIVE_URL=http://localhost:<port> npm run nala local`
(or use the parallel 'wt test' wrapper). Forgetting pointed tests
at the main repo on 3000 and silently ran against the wrong branch.

After: 'npm run nala local' Just Works from any worktree.
Copy link
Copy Markdown
Collaborator

@Roycethan Roycethan left a comment

Choose a reason for hiding this comment

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

@Axelcureno Collections tab is returning 0 results in workspaces like Acom, Nala etc...
ex: https://mwpw-191570--mas--adobecom.aem.live/studio.html?#page=translation-editor&path=acom

Image

The Select items dialog partitioned collections from the shared
paginated card stream, but on surfaces where cards dominate the first
pages (acom, nala), the initial fetch returned zero collections and
the Collections tab showed 'No items found' until the user scrolled
enough to drag collections in.

Add repository.loadAllCollections() that fires a one-shot
model-filtered query scoped to the current surface/locale (limit 50
per page — higher limits return 400 from AEM). Wire it into the
translation editor's connectedCallback alongside searchFragments
and loadPlaceholders. Update loadAllFragments to no-op for the
Collections tab so the dedicated query owns the store.

Bench: acom surface, 108 collections appear immediately instead of
0. Sandbox/nala (small surfaces) unaffected.
- studioPath now uses getFragmentName(fragment) so collections render
  the same human-readable label ("merch-card-collection: ACOM / …")
  that cards already use via translation-items-loader. Previously
  showed raw DAM paths — UX regression the select-items dialog
  wouldn't have surfaced until a content-heavy surface landed.
- Drop the redundant client-side model.path filter; AEM's modelIds
  query already scopes to collections.
- Build the collections array and the path->collection Map in a
  single pass instead of iterating the fragments three times.
Fragment 52b30904-c8b1-44d2-ada4-cbbdf87b7603's French localized
description field renders 'Voir les conditions' (verified against
the fragment API), matching the canonical label already used by
@MAS-CCD-mini-card-fr and @MAS-CCD-mini-card-fr_promo in the same
spec. Only @MAS-CCD-mini-card-fr-team kept the stale
'Conditions d'utilisation'; bringing it in line unblocks the Docs
NALA gate.
@studio-translations-list-load has been flaky on CI, failing with 0
rows because the translations table emits 'visible' before the
async row population completes. Extending the wait window gives the
list time to populate with the expected 'loc 1'/'loc 2'/'loc 3'
fixture rows in slower CI runs.
waitForListToLoad previously resolved as soon as the translation
table became visible, which happens during skeleton shimmer before
the async row population finishes. The @studio-translations-list-load
test kept failing with rowCount=0 because it ran assertions against
the still-skeleton table.

Use locator.or() to wait for either an sp-table-row that is NOT a
skeleton row, or the no-results empty state — whichever appears
first. Guarantees the table has settled into its final state before
tests assert on rows.
Comment thread nala/docs/ccd-mini/mini.spec.js Outdated
recurrenceText: '/mois',
seeTerms: {
text: 'Conditions d’utilisation',
text: 'Voir les conditions',
Copy link
Copy Markdown
Contributor

@afmicka afmicka Apr 17, 2026

Choose a reason for hiding this comment

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

please remove this change unless you have a reason why you are changing the placeholder value in the Nala surface. Lets discuss if the change is really needed. For now i am setting the previous value

Image

@Roycethan
Copy link
Copy Markdown
Collaborator

when more number of  fragments selected , i see the 'Hide selection' button is bit overlapping over the right pane items
image

The side panel's <ul.selected-items> had no height constraint or
overflow, so with 7+ selected items the list grew past the dialog's
flex container and overlapped the Cancel / Add selected items
footer. Sibling mas-select-items-table cooperated with the parent's
height budget via flex:1/min-height:0; mas-selected-items did not.

Add min-height:0 + max-height:100% on :host so the component
respects the parent container's bounds, and overflow-y:auto +
min-height:0 on .selected-items so the list scrolls internally
when it outgrows the available space.
Dropping the CCD mini spec change from this PR's scope — it's a
content/spec drift on ACOM docs unrelated to the translation editor
work this PR covers. The failing test will be addressed in a
separate PR owned by the CCD mini card team.

This reverts commit f589a4f.
@Axelcureno Axelcureno requested a review from Roycethan April 17, 2026 15:58
@Roycethan Roycethan merged commit 4b1efaa into main Apr 17, 2026
11 of 13 checks passed
@Roycethan Roycethan deleted the MWPW-191570 branch April 17, 2026 16:34
Axelcureno added a commit that referenced this pull request Apr 20, 2026
Sync with origin/main (26 commits). Accepted main's translation
refactor and io/studio additions (PRs #726, #742, #762, #764).
Merged surface-scoped settings access (PR #733) with branch's
getUserSurfaces; dropped unused isPowerUser. Merged locale picker
fix (PR #672) with branch's product-catalog additions in mas-top-nav
and router. Kept branch's new OST bundle pending full integration.
Dropped web-components/dist conflicts and rebuilt.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants