Merge pull request #11366 from eth3lbert/version-actions-menu

Replace yank button with a drop down menu

Author: Turbo87

app/components/privileged-action.hbs

@@ -1,14 +1,14 @@
 {{#if this.isPrivileged}}
-  <div>
+  <div ...attributes>
     {{yield}}
   </div>
 {{else if this.canBePrivileged}}
   {{#if (has-block 'placeholder')}}
-    <div>
+    <div ...attributes>
       {{yield to='placeholder'}}
     </div>
   {{else}}
-    <div local-class='placeholder'>
+    <div local-class='placeholder' ...attributes>
       <fieldset data-test-placeholder-fieldset disabled="disabled">
         {{yield}}
       </fieldset>
@@ -18,7 +18,7 @@
     </div>
   {{/if}}
 {{else}}
-  <div>
+  <div ...attributes>
     {{#if (has-block 'unprivileged')}}
       {{yield to='unprivileged'}}
     {{/if}}

app/components/version-list/row.hbs

@@ -117,7 +117,18 @@
     {{/if}}
   </div>
 
-  <PrivilegedAction @userAuthorised={{this.isOwner}}>
-    <YankButton @version={{@version}} local-class="yank-button" />
+  <PrivilegedAction @userAuthorised={{this.isOwner}} local-class="actions">
+    <Dropdown local-class="dropdown" data-test-actions-menu as |dd|>
+      <dd.Trigger @hideArrow={{true}} local-class="trigger" data-test-actions-toggle>
+        {{svg-jar "ellipsis-circle" local-class="icon"}}
+        <span class="sr-only">Actions</span>
+      </dd.Trigger>
+
+      <dd.Menu local-class="menu" as |menu|>
+        <menu.Item>
+          <YankButton @version={{@version}} class="button-reset" local-class="menu-button" />
+        </menu.Item>
+      </dd.Menu>
+    </Dropdown>
   </PrivilegedAction>
 </div>

app/components/version-list/row.module.css

@@ -235,11 +235,52 @@
     margin-top: var(--space-2xs);
 }
 
-.yank-button {
-    position: relative;
-    margin-left: var(--space-xs);
+.actions {
+    display: flex;
+}
 
-    @media only screen and (max-width: 550px) {
-        display: none;
+.dropdown {
+    display: flex;
+    font-size: initial;
+    line-height: 1rem;
+}
+
+.icon {
+    width: 2em;
+    height: auto;
+}
+
+.trigger {
+    background: none;
+    border: none;
+    padding: 0;
+    border-radius: 99999px;
+    color: var(--grey600);
+
+    :hover {
+        border-radius: 99999px;
+        color: var(--grey900);
+        background-color: white;
+    }
+}
+
+.menu {
+    top: 100%;
+    right: 0;
+    min-width: max-content;
+}
+
+.menu-button {
+    align-items: center;
+    gap: var(--space-2xs);
+    cursor: pointer;
+    text-transform: capitalize;
+
+    /* This duplicates the styles in .button[disabled] as there's no
+     * obvious way to compose them, given the target selectors. */
+    &[disabled] {
+        background: linear-gradient(to bottom, var(--bg-color-top-light) 0%, var(--bg-color-bottom-light) 100%);
+        color: var(--disabled-text-color) !important;
+        cursor: not-allowed;
     }
 }

app/components/yank-button.hbs

@@ -1,7 +1,6 @@
 {{#if @version.yanked}}
   <button
     type="button"
-    class="button button--small"
     ...attributes
     data-test-version-unyank-button={{@version.num}}
     disabled={{@version.unyankTask.isRunning}}
@@ -16,7 +15,6 @@
 {{else}}
   <button
     type="button"
-    class="button button--small"
     ...attributes
     data-test-version-yank-button={{@version.num}}
     disabled={{@version.yankTask.isRunning}}

e2e/acceptance/sudo.spec.ts

@@ -36,6 +36,8 @@ test.describe('Acceptance | sudo', { tag: '@acceptance' }, () => {
     await expect(page.locator('[data-test-disable-admin-actions]')).toHaveCount(0);
     await expect(page.locator('[data-test-enable-admin-actions]')).toHaveCount(0);
 
+    // Assert that there's no dropdown menu toggle, disabled, enabled, or in any state.
+    await expect(page.locator('[data-test-actions-toggle]')).toHaveCount(0);
     // Assert that there's no yank button, disabled, enabled, or in any state.
     await expect(page.locator('[data-test-version-yank-button="0.1.0"]')).toHaveCount(0);
   });
@@ -53,14 +55,13 @@ test.describe('Acceptance | sudo', { tag: '@acceptance' }, () => {
     await expect(page.locator('[data-test-enable-admin-actions]')).toBeVisible();
 
     // Test that the fieldset is present and disabled.
-    await expect(page.locator('[data-test-placeholder-fieldset]')).toBeVisible();
+    await expect(page.locator('[data-test-placeholder-fieldset]').first()).toBeVisible();
     // NOTE: `toBeDisabled()` is not working as expected because the element is not a form control element.
     // Ref: https://github.com/microsoft/playwright/issues/13583#issuecomment-1101704985
-    await expect(page.locator('[data-test-placeholder-fieldset]')).toHaveAttribute('disabled', 'disabled');
+    await expect(page.locator('[data-test-placeholder-fieldset]').first()).toHaveAttribute('disabled', 'disabled');
 
-    // From the perspective of the actual button, it isn't disabled, even though
-    // the fieldset effectively makes it unclickable.
-    await expect(page.locator('[data-test-version-yank-button="0.1.0"]')).toBeVisible();
+    await expect(page.locator('[data-test-actions-toggle]')).toBeDisabled();
+    await expect(page.locator('[data-test-version-yank-button="0.1.0"]')).toBeHidden();
   });
 
   test('admin user can enter sudo mode', async ({ page, msw }) => {
@@ -93,6 +94,8 @@ test.describe('Acceptance | sudo', { tag: '@acceptance' }, () => {
     });
     expect(seen).toBe(1);
 
+    await page.locator('[data-test-actions-toggle]').click();
+
     // Test that the fieldset is not present.
     await expect(page.locator('[data-test-placeholder-fieldset]')).toHaveCount(0);
     await expect(page.locator('[data-test-version-yank-button="0.1.0"]')).toBeVisible();
@@ -106,6 +109,8 @@ test.describe('Acceptance | sudo', { tag: '@acceptance' }, () => {
     await page.locator('[data-test-user-menu]').getByRole('button').click();
     await page.getByRole('button', { name: 'Enable admin actions' }).click();
 
+    await page.locator('[data-test-actions-toggle]').click();
+
     const yankButton = page.locator('[data-test-version-yank-button="0.1.0"]');
     const unyankButton = page.locator('[data-test-version-unyank-button="0.1.0"]');
 

e2e/acceptance/versions.spec.ts

@@ -52,6 +52,8 @@ test.describe('Acceptance | crate versions page', { tag: '@acceptance' }, () =>
     await expect(v020).not.toHaveClass(/.*latest/);
     await expect(v020).not.toHaveClass(/.yanked/);
 
+    await v021.locator('[data-test-actions-toggle]').click();
+
     // yanking
     await page.locator('[data-test-version-yank-button="0.2.1"]').click();
     await expect(v021).not.toHaveClass(/.*latest/);

public/assets/ellipsis-circle.svg

@@ -43,6 +43,8 @@ module('Acceptance | sudo', function (hooks) {
     assert.dom('[data-test-disable-admin-actions]').doesNotExist();
     assert.dom('[data-test-enable-admin-actions]').doesNotExist();
 
+    // Assert that there's no dropdown menu toggle, disabled, enabled, or in any state.
+    assert.dom('[data-test-actions-toggle]').doesNotExist();
     // Assert that there's no yank button, disabled, enabled, or in any state.
     assert.dom('[data-test-version-yank-button="0.1.0"]').doesNotExist();
   });
@@ -62,6 +64,7 @@ module('Acceptance | sudo', function (hooks) {
 
     // From the perspective of the actual button, it isn't disabled, even though
     // the fieldset effectively makes it unclickable.
+    assert.dom('[data-test-actions-toggle]').exists();
     assert.dom('[data-test-version-yank-button="0.1.0"]').exists();
   });
 
@@ -90,6 +93,8 @@ module('Acceptance | sudo', function (hooks) {
     }
     assert.strictEqual(seen, 1);
 
+    await click('[data-test-actions-toggle]');
+
     // Test that the fieldset is not present.
     assert.dom('[data-test-placeholder-fieldset]').doesNotExist();
     assert.dom('[data-test-version-yank-button="0.1.0"]').exists();
@@ -101,6 +106,8 @@ module('Acceptance | sudo', function (hooks) {
     await visit('/crates/foo/versions');
     await click('[data-test-enable-admin-actions]');
 
+    await click('[data-test-actions-toggle]');
+
     await click('[data-test-version-yank-button="0.1.0"]');
 
     await waitFor('[data-test-version-unyank-button="0.1.0"]');

tests/acceptance/sudo-test.js

@@ -57,6 +57,8 @@ module('Acceptance | crate versions page', function (hooks) {
       .hasNoClass(/.*latest/)
       .hasNoClass(/.yanked/);
 
+    await click('[data-test-actions-toggle]');
+
     // yanking
     await click('[data-test-version-yank-button="0.2.1"]');
     assert