Team management UI improvements (#6699)

* Pin prima to a git ref for in-progress Listbox work

Also, temporarily make sure prima.js is built on CI

* introduce PrimaListbox and use it in role_picker

* rename role_picker to role_select_input

* team management: extract remove member from dropdown and use PrimaListbox

* a few styling improvements

* Improve team management UI mobile layout and pending avatar styling

* re-apply Sanne's changes to PrimaListbox

Pulled these changes out in order to merge #6706 first. Re-applying
them now.

* Use phx-change instead of phx-click (role_switcher)

* Remove unnecessary input_role server state and click handler

* rely on Prima's disabled option

* make sure screen readers announce listbox properly

* include user email to acessible names per member row

* fix confirmation dialog selection bug

* do not disable currently selected role option

* make disabled remove member button gray instead of faded-red

* simplify applying disabled classes

Make the global component styles in app.css respect data-disabled (used
for disabling components in Prima) when applying disabled classes.

* remove redundant phx-value-name

* npm lint assets

* fix mix.lock after merge commit

* update Prima pin

* move disabled attribute to listbox wrapper + update disabled variant

* update Prima pin

* fix matching trigger width

* update Prima pin

* add selected check icon for role_select_input too

* move Prima dependency back to Hex and revert its build orchestration

* revert unrelated mix.lock changes

---------

Co-authored-by: Sanne de Vries <sannedv@protonmail.com>

Author: RobertJoonas

assets/css/app.css

@@ -303,6 +303,11 @@
 @custom-variant phx-submit-loading (.phx-submit-loading&, .phx-submit-loading &);
 @custom-variant phx-change-loading (.phx-change-loading&, .phx-change-loading &);
 
+/* Prima components usually mark their disabled state with `data-disabled` rather than
+   the native attribute, so `disabled:` covers both. */
+
+@custom-variant disabled (&:disabled, &[data-disabled="true"], [data-disabled="true"] &);
+
 [x-cloak] {
   display: none;
 }

assets/js/liveview/confirm.js

@@ -0,0 +1,27 @@
+// An alternative for phoenix_html's `data-confirm`. Currently delegates to
+// `window.confirm` just as `data-confirm` does, but can be extended with custom
+// styling in the future.
+
+// Unlike `data-confirm` (which uses a window-level click listener), this hook
+// intercepts the click on the element itself, so it also works when a listener
+// closer to the element acts on the click first - e.g. a Prima listbox selecting
+// the clicked option.
+
+export default {
+  mounted() {
+    this.handleClick = (e) => {
+      const message = this.el.dataset.confirmMessage
+
+      if (message && !window.confirm(message)) {
+        e.preventDefault()
+        e.stopImmediatePropagation()
+      }
+    }
+
+    this.el.addEventListener('click', this.handleClick, true)
+  },
+
+  destroyed() {
+    this.el.removeEventListener('click', this.handleClick, true)
+  }
+}

assets/js/liveview/live_socket.js

@@ -7,19 +7,20 @@
 import 'phoenix_html'
 import { Socket } from 'phoenix'
 import { LiveSocket } from 'phoenix_live_view'
-import { Modal, Dropdown } from 'prima'
+import { Modal, Dropdown, Listbox } from 'prima'
 import topbar from 'topbar'
 /* eslint-enable import/no-unresolved */
 
 import Alpine from 'alpinejs'
 
+import Confirm from './confirm'
 import CopySnippet from './copy-snippet'
 import MemberRows from './member-rows'
 
 let csrfToken = document.querySelector("meta[name='csrf-token']")
 let websocketUrl = document.querySelector("meta[name='websocket-url']")
 if (csrfToken && websocketUrl) {
-  let Hooks = { Modal, Dropdown, CopySnippet, MemberRows }
+  let Hooks = { Modal, Dropdown, Listbox, Confirm, CopySnippet, MemberRows }
 
   Hooks.VerificationLifecycle = {
     mounted() {

e2e/tests/dashboard/team-management.spec.ts

@@ -0,0 +1,179 @@
+import { test, expect, Page } from '@playwright/test'
+import { setupSite } from '../fixtures'
+import {
+  closeFlashMessage,
+  expectLiveViewConnected,
+  hasFlashMessage,
+  hasNoFlashMessage,
+  randomID
+} from '../test-utils'
+
+async function createTeam(page: Page) {
+  await page.goto('/team/setup', { waitUntil: 'commit' })
+  await expectLiveViewConnected(page)
+  await page.getByRole('button', { name: 'Create team' }).click()
+  await expect(page).toHaveURL(/\/settings\/team\/general/)
+  await expectLiveViewConnected(page)
+  // so that tests start without a flash
+  await closeFlashMessage(page)
+}
+
+async function submitInviteForm(
+  page: Page,
+  { email, roleOptionName }: { email: string; roleOptionName: string | RegExp }
+) {
+  await page.getByPlaceholder('Enter e-mail').fill(email)
+  await page.locator('#input-role-picker-trigger').click()
+  await page
+    .locator('#input-role-picker')
+    .getByRole('option', { name: roleOptionName })
+    .click()
+  await page.getByRole('button', { name: 'Invite' }).click()
+}
+
+test('lowering your own role asks for confirmation', async ({
+  page,
+  request
+}) => {
+  const { user } = await setupSite({ page, request })
+  await createTeam(page)
+
+  // The only owner can't change their own role, so a second (invited) owner
+  // is needed for the self-demotion options to become selectable.
+  const ownerEmail = `owner-${randomID()}@example.com`
+  await submitInviteForm(page, { email: ownerEmail, roleOptionName: /^Owner/ })
+  await expect(
+    page.getByRole('button', { name: `Role for ${ownerEmail}:` })
+  ).toContainText('Owner')
+
+  const trigger = page.getByRole('button', { name: `Role for ${user.email}:` })
+  const options = page.getByRole('listbox', {
+    name: `Role for ${user.email}:`
+  })
+
+  const dialogs: string[] = []
+  const answerNextDialog = (accept: boolean) =>
+    page.once('dialog', async (dialog) => {
+      dialogs.push(dialog.message())
+      await (accept ? dialog.accept() : dialog.dismiss())
+    })
+
+  await expect(trigger).toBeEnabled()
+  await expect(trigger).toContainText('Owner')
+
+  await test.step('lower your own role, but cancel the confirmation dialog (mouse)', async () => {
+    answerNextDialog(false)
+
+    await trigger.click()
+    await options.getByRole('option', { name: /^Viewer/ }).click()
+
+    expect(dialogs).toHaveLength(1)
+    expect(dialogs[0]).toContain("You're about to lower your own role")
+    await expect(trigger).toContainText('Owner')
+    await hasNoFlashMessage(page)
+
+    // Close the listbox that remained open because the option selection click didn't
+    // go through -- our confirmation dialog prevented it from bubbling up to Prima.
+    await page.keyboard.press('Escape')
+  })
+
+  await test.step('lower your own role, but cancel the confirmation dialog (keyboard)', async () => {
+    answerNextDialog(false)
+
+    await trigger.focus()
+    await page.keyboard.press('ArrowUp')
+    await expect(
+      options.getByRole('option', { name: /^Viewer/ })
+    ).toHaveAttribute('data-focus')
+    await page.keyboard.press('Enter')
+
+    expect(dialogs).toHaveLength(2)
+    await expect(trigger).toContainText('Owner')
+    await hasNoFlashMessage(page)
+
+    // Close the listbox that remained open because the option selection click didn't
+    // go through -- our confirmation dialog prevented it from bubbling up to Prima.
+    await page.keyboard.press('Escape')
+  })
+
+  await test.step('lower your own role and confirm (mouse)', async () => {
+    answerNextDialog(true)
+
+    await trigger.click()
+    await options.getByRole('option', { name: /^Viewer/ }).click()
+
+    // The change reaches the server, which rejects it: the other owner has only
+    // been invited so far.
+    await hasFlashMessage(page, 'The team has to have at least one owner')
+
+    // asked once: phoenix_html must not ask again as the confirmed click bubbles up
+    expect(dialogs).toHaveLength(3)
+
+    // Known bug, to be fixed soon: the trigger keeps showing the rejected role
+    // until the page is reloaded. Once fixed, the reload won't be necessary.
+    await page.reload()
+    await expectLiveViewConnected(page)
+
+    await expect(trigger).toContainText('Owner')
+  })
+
+  await test.step('lower your own role and confirm (keyboard)', async () => {
+    answerNextDialog(true)
+
+    await trigger.focus()
+    await page.keyboard.press('ArrowUp')
+    await expect(
+      options.getByRole('option', { name: /^Viewer/ })
+    ).toHaveAttribute('data-focus')
+    await page.keyboard.press('Enter')
+
+    // The change reaches the server, which rejects it: the other owner has only
+    // been invited so far.
+    await hasFlashMessage(page, 'The team has to have at least one owner')
+
+    // asked once: phoenix_html must not ask again as the confirmed click bubbles up
+    expect(dialogs).toHaveLength(4)
+
+    // Known bug, to be fixed soon: the trigger keeps showing the rejected role
+    // until the page is reloaded. Once fixed, the reload won't be necessary.
+    await page.reload()
+    await expectLiveViewConnected(page)
+
+    await expect(trigger).toContainText('Owner')
+  })
+})
+
+test('choosing the current role again just closes the listbox', async ({
+  page,
+  request
+}) => {
+  await setupSite({ page, request })
+  await createTeam(page)
+
+  const viewerEmail = `viewer-${randomID()}@example.com`
+  await submitInviteForm(page, {
+    email: viewerEmail,
+    roleOptionName: /^Viewer/
+  })
+
+  const trigger = page.getByRole('button', { name: `Role for ${viewerEmail}:` })
+  const options = page.getByRole('listbox', {
+    name: `Role for ${viewerEmail}:`
+  })
+
+  await expect(trigger).toContainText('Viewer')
+
+  const dialogs: string[] = []
+  page.on('dialog', async (dialog) => {
+    dialogs.push(dialog.message())
+    await dialog.dismiss()
+  })
+
+  await trigger.click()
+  await options.getByRole('option', { name: /^Viewer/ }).click()
+
+  await expect(trigger).toHaveAttribute('aria-expanded', 'false')
+  await expect(trigger).toContainText('Viewer')
+  expect(dialogs).toHaveLength(0)
+  await hasNoFlashMessage(page)
+})

e2e/tests/test-utils.ts

@@ -21,6 +21,25 @@ export async function expectLiveViewConnected(page: Page) {
     .toBeGreaterThan(0)
 }
 
+const visibleFlash = (page: Page) =>
+  page.getByTestId('live-flash').filter({ visible: true })
+
+export async function hasFlashMessage(page: Page, message?: string) {
+  const flash = visibleFlash(page)
+  await expect(
+    message ? flash.filter({ hasText: message }) : flash
+  ).toBeVisible()
+}
+
+export async function hasNoFlashMessage(page: Page) {
+  await expect(visibleFlash(page)).toHaveCount(0)
+}
+
+export async function closeFlashMessage(page: Page) {
+  await visibleFlash(page).getByRole('button').click()
+  await hasNoFlashMessage(page)
+}
+
 export function randomID() {
   return Math.random().toString(16).slice(2)
 }

lib/plausible_web/components/prima_dropdown.ex

@@ -64,8 +64,7 @@ defmodule PlausibleWeb.Components.PrimaDropdown do
     <Dropdown.dropdown_menu
       id={@id}
       placement="bottom-end"
-      match_trigger_width={true}
-      class="bg-white rounded-md shadow-lg ring-1 ring-black/5 focus:outline-none p-1.5 dark:bg-gray-800 relative z-10"
+      class="min-w-[var(--reference-width)] bg-white rounded-md shadow-lg ring-1 ring-black/5 focus:outline-none p-1.5 dark:bg-gray-800 relative z-10"
     >
       {render_slot(@inner_block)}
     </Dropdown.dropdown_menu>