Settings, Vault shows your vaults and Manage… again

src/ui-mobile/MobileShell.tsx

@@ -57,7 +57,7 @@ import ensoUrl from '../assets/enso.png'
 import { getStoragePref } from '../bridge/icloud'
 import { backgroundSyncEnabled, setBackgroundSyncEnabled } from '../bridge/background-sync'
 import { getPinnedNotes, loadPins } from './pins'
-import { vaultSettingsAccessForLayout } from './vault-settings-access'
+import { placeVaultSettingsIsland, vaultSettingsAccessForLayout } from './vault-settings-access'
 import { getLayoutMode, isPhoneViewport, setLayoutMode, type LayoutMode } from '../viewport'
 import {
   applySystemBarsPreference,
@@ -2436,7 +2436,10 @@ function useVaultSettingsRows(): void {
       // which reads as the whole card blinking. Moving the same container
       // back in preserves the live React tree and its state.
       if (!host) return
-      if (container?.parentElement === host) return
+      // contains, not parentElement: on a tablet the container lives in the
+      // row's button group, and re-placing it on every mutation would fire
+      // this observer again forever.
+      if (container && host.contains(container)) return
       if (!container) {
         container = document.createElement('div')
         // The phone wrapper — not the list inside it — is the row's flex child,
@@ -2452,18 +2455,7 @@ function useVaultSettingsRows(): void {
           access === 'quick-switch' ? <SettingsVaultQuickSwitch /> : <SettingsVaultManageButton />
         )
       }
-      if (access === 'quick-switch') {
-        // The host is the desktop two-column row (label + "Change…" button),
-        // which the phone stylesheet wraps into a stack. Mount between the
-        // label and the row's own buttons, so the card reads location →
-        // picker list → actions and mobile.css can hide those buttons in
-        // favor of the island's own action group.
-        const firstBtn = host.querySelector('button')
-        if (firstBtn) host.insertBefore(container, firstBtn)
-        else host.appendChild(container)
-      } else {
-        host.appendChild(container)
-      }
+      placeVaultSettingsIsland(host, container, access)
     }
     const observer = new MutationObserver(() => sync())
     observer.observe(document.body, { childList: true, subtree: true })

src/ui-mobile/mobile.css

@@ -2130,35 +2130,32 @@
 /* ---- Settings vault quick-switch (useVaultSettingsRows) ------------------- */
 
 /* Sits inside the location row (stacked on the phone), between the label and
-   the row's own buttons — the row's px-5 padding is already around it. */
+   the row's own actions; the row's px-5 padding is already around it. */
 .zn-settings-vaults-host {
   flex-basis: 100%;
   min-width: 100%;
 }
 
-/* In the tablet settings row, keep only the compact manager entry point. The
-   wrapper disappears from flex layout so its button sits beside Change…
-   instead of forcing the phone quick-switch card into the row. */
+/* In the tablet settings row, keep only the compact manager entry point.
+   placeVaultSettingsIsland puts the wrapper at the end of the row's button
+   group, and the wrapper disappears from flex layout so its button sits
+   beside Change… and Quick Connect… and wraps with them (core wraps that
+   group under the label on its own since 2.58.0). */
 .zn-settings-vaults-manage-host {
   display: contents;
 }
 
-[data-settings-search-id='vault-location']:has(> .zn-settings-vaults-manage-host) {
-  flex-wrap: wrap;
-  row-gap: 0.5rem;
-}
-
 .zn-settings-vaults {
   padding-top: 0.625rem;
   border-top: 0.5px solid rgb(var(--z-bg-4) / 0.55);
 }
 
-/* The card's own actions ("Change…", and in a remote workspace its siblings)
-   are hidden while the island is mounted — the island's action group renders
-   its own Change… (same store flows) so all actions live in ONE clearly
-   separated button row under the picker list (Adib: buttons must read apart
-   from the list). Scoped with :has() so this only applies where the island
-   actually mounted (phone width). */
+/* The row's own actions (Change…, and in a remote workspace its siblings)
+   are hidden while the island is mounted: the island's action group carries
+   New Vault… and Manage…, so all actions live in ONE clearly separated
+   button row under the picker list (Adib: buttons must read apart from the
+   list). Scoped with :has() so this only applies where the island actually
+   mounted (phone layout). */
 .zn-mobile
   [data-zn-settings]
   div.flex.items-center.justify-between[class*='px-']:has(> .zn-settings-vaults-host) {
@@ -2170,7 +2167,12 @@
   padding-bottom: 0.625rem;
 }
 
-[data-settings-search-id='vault-location']:has(> .zn-settings-vaults-host) > button {
+/* Everything after the island that is or holds a button is the row's own
+   actions: bare buttons before core 2.58.0, one wrapping group since (#871).
+   A rule for bare buttons alone stopped matching when core wrapped them. */
+[data-settings-search-id='vault-location']
+  > .zn-settings-vaults-host
+  ~ :is(button, :has(button)) {
   display: none;
 }
 

src/ui-mobile/vault-settings-access.test.ts

@@ -1,6 +1,10 @@
 import assert from 'node:assert/strict'
 import { describe, it } from 'node:test'
-import { vaultSettingsAccessForLayout } from './vault-settings-access.ts'
+import {
+  placeVaultSettingsIsland,
+  vaultRowActionGroup,
+  vaultSettingsAccessForLayout
+} from './vault-settings-access.ts'
 
 describe('vaultSettingsAccessForLayout', () => {
   it('keeps the full quick-switch controls in the phone settings layout', () => {
@@ -11,3 +15,146 @@ describe('vaultSettingsAccessForLayout', () => {
     assert.equal(vaultSettingsAccessForLayout(false), 'manage-button')
   })
 })
+
+// The tests run under plain node, so the row is a small stand-in for the DOM
+// that keeps the rule that broke the island: insertBefore throws unless the
+// reference node is a direct child.
+class FakeElement {
+  readonly tagName: string
+  readonly label: string
+  parentElement: FakeElement | null = null
+  private kids: FakeElement[] = []
+
+  constructor(tagName: string, label: string, kids: FakeElement[] = []) {
+    this.tagName = tagName.toUpperCase()
+    this.label = label
+    for (const kid of kids) this.appendChild(kid)
+  }
+
+  get children(): FakeElement[] {
+    return [...this.kids]
+  }
+
+  querySelector(selector: string): FakeElement | null {
+    assert.equal(selector, 'button')
+    for (const kid of this.kids) {
+      if (kid.tagName === 'BUTTON') return kid
+      const found = kid.querySelector(selector)
+      if (found) return found
+    }
+    return null
+  }
+
+  appendChild(node: FakeElement): FakeElement {
+    return this.insertBefore(node, null)
+  }
+
+  insertBefore(node: FakeElement, ref: FakeElement | null): FakeElement {
+    if (ref && ref.parentElement !== this) {
+      throw new Error(
+        'NotFoundError: The node before which the new node is to be inserted is not a child of this node.'
+      )
+    }
+    node.parentElement?.removeChild(node)
+    const at = ref ? this.kids.indexOf(ref) : this.kids.length
+    this.kids.splice(at, 0, node)
+    node.parentElement = this
+    return node
+  }
+
+  removeChild(node: FakeElement): void {
+    this.kids = this.kids.filter((kid) => kid !== node)
+    node.parentElement = null
+  }
+}
+
+const el = (tag: string, label: string, kids: FakeElement[] = []): FakeElement =>
+  new FakeElement(tag, label, kids)
+const labels = (node: FakeElement): string[] => node.children.map((kid) => kid.label)
+const place = (row: FakeElement, island: FakeElement, phone: boolean): void =>
+  placeVaultSettingsIsland(
+    row as unknown as Element,
+    island as unknown as Element,
+    vaultSettingsAccessForLayout(phone)
+  )
+
+// SettingsModal's Vault Location row since core 2.58.0 (#871): the label,
+// then ONE wrapping group that holds the buttons.
+const coreRow = (buttons: string[]): FakeElement =>
+  el('div', 'row', [
+    el('div', 'label', [el('div', 'title'), el('div', 'path')]),
+    el(
+      'div',
+      'actions',
+      buttons.map((name) => el('button', name))
+    )
+  ])
+
+// The same row before 2.58.0: the buttons sat bare in the row.
+const bareRow = (buttons: string[]): FakeElement =>
+  el('div', 'row', [el('div', 'label'), ...buttons.map((name) => el('button', name))])
+
+const phoneIsland = (): FakeElement =>
+  el('div', 'island', [el('button', 'My Vault'), el('div', 'btns', [el('button', 'New Vault…')])])
+
+describe('placeVaultSettingsIsland on the phone', () => {
+  it('mounts between the label and the wrapped button group', () => {
+    const row = coreRow(['Change…'])
+    const island = phoneIsland()
+    place(row, island, true)
+    assert.deepEqual(labels(row), ['label', 'island', 'actions'])
+  })
+
+  it('mounts the same way in a remote workspace, with four buttons', () => {
+    const row = coreRow([
+      'Change Remote Vault…',
+      'Return to Local Vault',
+      'Open Local Vault…',
+      'Quick Connect…'
+    ])
+    const island = phoneIsland()
+    place(row, island, true)
+    assert.deepEqual(labels(row), ['label', 'island', 'actions'])
+  })
+
+  it('still mounts before bare buttons', () => {
+    const row = bareRow(['Change…'])
+    const island = phoneIsland()
+    place(row, island, true)
+    assert.deepEqual(labels(row), ['label', 'island', 'Change…'])
+  })
+
+  it('appends when the row has no buttons at all', () => {
+    const row = el('div', 'row', [el('div', 'label')])
+    const island = phoneIsland()
+    place(row, island, true)
+    assert.deepEqual(labels(row), ['label', 'island'])
+  })
+})
+
+describe('placeVaultSettingsIsland on a tablet', () => {
+  it('joins the button group so Manage… sits beside Change…', () => {
+    const row = coreRow(['Change…'])
+    const island = el('div', 'manage', [el('button', 'Manage…')])
+    place(row, island, false)
+    assert.deepEqual(labels(row), ['label', 'actions'])
+    assert.deepEqual(labels(row.children[1]), ['Change…', 'manage'])
+  })
+
+  it('appends to the row itself when the buttons are bare', () => {
+    const row = bareRow(['Change…'])
+    const island = el('div', 'manage', [el('button', 'Manage…')])
+    place(row, island, false)
+    assert.deepEqual(labels(row), ['label', 'Change…', 'manage'])
+  })
+})
+
+describe('vaultRowActionGroup', () => {
+  it("never mistakes the island's own buttons for the row's", () => {
+    const row = coreRow(['Change…'])
+    const island = phoneIsland()
+    row.insertBefore(island, row.children[1])
+    const group = vaultRowActionGroup(row as unknown as Element, island as unknown as Element)
+    assert.equal((group as unknown as FakeElement | null)?.label, 'actions')
+  })
+})

src/ui-mobile/vault-settings-access.ts

@@ -5,3 +5,38 @@ export type VaultSettingsAccess = 'quick-switch' | 'manage-button'
 export function vaultSettingsAccessForLayout(isPhoneLayout: boolean): VaultSettingsAccess {
   return isPhoneLayout ? 'quick-switch' : 'manage-button'
 }
+
+/** The Vault Location row's own actions: the first direct child of the row
+ * that is, or holds, a button. Core has shipped both shapes, bare buttons in
+ * the row before 2.58.0 and one wrapping button group since (#871). Looking
+ * for a direct child, not the first button, is what keeps the island
+ * mountable: a nested button is not a child of the row, insertBefore throws
+ * on it, and the island silently never appeared on the phone. The island is
+ * skipped so its own buttons never count as the row's. */
+export function vaultRowActionGroup(row: Element, island: Element): Element | null {
+  for (const child of Array.from(row.children)) {
+    if (child === island) continue
+    if (child.tagName === 'BUTTON' || child.querySelector('button')) return child
+  }
+  return null
+}
+
+/** Mount a vault island into the Vault Location row.
+ *
+ * Phone: between the label and the row's actions, so the card reads
+ * location, picker list, actions, and mobile.css hides the row's actions
+ * after the island in favor of the island's own group.
+ *
+ * Tablet: at the end of the row's button group, so Manage… sits beside
+ * Change… instead of wrapping onto a line of its own. With bare buttons the
+ * row itself is the group. */
+export function placeVaultSettingsIsland(
+  row: Element,
+  island: Element,
+  access: VaultSettingsAccess
+): void {
+  const actions = vaultRowActionGroup(row, island)
+  if (access === 'quick-switch') row.insertBefore(island, actions)
+  else if (actions && actions.tagName !== 'BUTTON') actions.appendChild(island)
+  else row.appendChild(island)
+}