UX: shows the bookmark menu improvements

This commit adds a new option `@modalForMobile` for `<DMenu />` which allows to display a `<DModal />` when expanding a menu on mobile. This commit also adds a `@views` options to toasts which is an array accepting `['mobile', 'desktop']` and will control if the toast is show on desktop and/or mobile. Finally this commit allows to hide the progressBar even if the toast is set to `@autoClose=true`. This is controlled through the `@showProgressBar` option.
2024-11-26 10:50:26 -06:00 · 2024-04-08 08:18:50 +02:00 · 2024-04-08 08:18:50 +02:00 · 17c92b4b2a
commit 17c92b4b2a
parent fb5ae16630
19 changed files with 230 additions and 124 deletions
--- a/app/assets/javascripts/discourse/app/components/bookmark-menu.gjs
+++ b/app/assets/javascripts/discourse/app/components/bookmark-menu.gjs
@ -21,6 +21,8 @@ export default class BookmarkMenu extends Component {
  @service modal;
  @service currentUser;
  @service toasts;
+  @service site;
+
  @tracked quicksaved = false;

  bookmarkManager = this.args.bookmarkManager;
@ -42,7 +44,7 @@ export default class BookmarkMenu extends Component {
  }

  get existingBookmark() {
-    return this.bookmarkManager.trackedBookmark.id
+    return this.bookmarkManager.trackedBookmark?.id
      ? this.bookmarkManager.trackedBookmark
      : null;
  }
@ -86,7 +88,9 @@ export default class BookmarkMenu extends Component {
      // a bookmark, it switches to the other Edit/Delete menu.
      this.quicksaved = true;
      this.toasts.success({
+        showProgressBar: false,
        duration: 3000,
+        views: ["mobile"],
        data: { message: I18n.t("bookmarks.bookmarked_success") },
      });
    } catch (error) {
@ -123,7 +127,11 @@ export default class BookmarkMenu extends Component {
      this.bookmarkManager.afterDelete(response, this.existingBookmark.id);
      this.toasts.success({
        duration: 3000,
-        data: { message: I18n.t("bookmarks.deleted_bookmark_success") },
+        showProgressBar: false,
+        data: {
+          icon: "trash-alt",
+          message: I18n.t("bookmarks.deleted_bookmark_success"),
+        },
      });
    } catch (error) {
      popupAjaxError(error);
@ -145,6 +153,8 @@ export default class BookmarkMenu extends Component {
        await this.bookmarkManager.save();
        this.toasts.success({
          duration: 3000,
+          showProgressBar: false,
+          views: ["mobile"],
          data: { message: I18n.t("bookmarks.reminder_set_success") },
        });
      } catch (error) {
@ -156,6 +166,8 @@ export default class BookmarkMenu extends Component {
  }

  async _openBookmarkModal() {
+    this.dMenu.close();
+
    try {
      const closeData = await this.modal.show(BookmarkModal, {
        model: {
@ -179,7 +191,6 @@ export default class BookmarkMenu extends Component {
      {{didInsert this.setReminderShortcuts}}
      @identifier="bookmark-menu"
      @triggers={{array "click"}}
-      @arrow="true"
      class={{concatClass
        "bookmark widget-button btn-flat no-text btn-icon bookmark-menu__trigger"
        (if this.existingBookmark "bookmarked")
@ -189,6 +200,8 @@ export default class BookmarkMenu extends Component {
      @onClose={{this.onCloseMenu}}
      @onShow={{this.onShowMenu}}
      @onRegisterApi={{this.onRegisterApi}}
+      @modalForMobile={{true}}
+      @arrow={{false}}
    >
      <:trigger>
        {{#if this.existingBookmark.reminderAt}}
@ -199,18 +212,31 @@ export default class BookmarkMenu extends Component {
      </:trigger>
      <:content>
        <div class="bookmark-menu__body">
+
+          {{#unless this.showEditDeleteMenu}}
+            <div class="bookmark-menu__title">{{icon "check-circle"}}<span
+              >{{i18n "bookmarks.bookmarked_success"}}</span>
+            </div>
+          {{/unless}}
+
          {{#if this.showEditDeleteMenu}}
+            {{#if this.site.mobileView}}
+              <div class="bookmark-menu__title">{{icon "bookmark"}}<span>{{i18n
+                    "bookmarks.bookmark"
+                  }}</span>
+              </div>
+            {{/if}}
            <ul class="bookmark-menu__actions">
              <li class="bookmark-menu__row -edit" data-menu-option-id="edit">
                <DButton
                  @icon="pencil-alt"
                  @label="edit"
                  @action={{this.onEditBookmark}}
-                  @class="bookmark-menu__row-btn btn-flat"
+                  @class="bookmark-menu__row-btn btn-transparent"
                />
              </li>
              <li
-                class="bookmark-menu__row -remove"
+                class="bookmark-menu__row --remove"
                role="button"
                tabindex="0"
                data-menu-option-id="delete"
@ -219,7 +245,7 @@ export default class BookmarkMenu extends Component {
                  @icon="trash-alt"
                  @label="delete"
                  @action={{this.onRemoveBookmark}}
-                  @class="bookmark-menu__row-btn btn-flat"
+                  @class="bookmark-menu__row-btn btn-transparent btn-danger"
                />
              </li>
            </ul>
@ -237,7 +263,7 @@ export default class BookmarkMenu extends Component {
                    @label={{option.label}}
                    @translatedTitle={{this.reminderShortcutTimeTitle option}}
                    @action={{fn this.onChooseReminderOption option}}
-                    @class="bookmark-menu__row-btn btn-flat"
+                    @class="bookmark-menu__row-btn btn-transparent"
                  />
                </li>
              {{/each}}
--- a/app/assets/javascripts/discourse/tests/integration/components/float-kit/d-default-toast-test.js
+++ b/app/assets/javascripts/discourse/tests/integration/components/float-kit/d-default-toast-test.js
@ -27,10 +27,9 @@ module(

    test("progress bar", async function (assert) {
      this.toast = new DToastInstance(this, {});
-      this.noop = () => {};

      await render(
-        hbs`<DDefaultToast @data={{this.toast.options.data}} @autoClose={{true}} @onRegisterProgressBar={{this.noop}} />`
+        hbs`<DDefaultToast @data={{this.toast.options.data}} @showProgressBar={{true}} @autoClose={{true}} @onRegisterProgressBar={{(noop)}} />`
      );

      assert.dom(".fk-d-default-toast__progress-bar").exists();
@ -40,7 +39,7 @@ module(
      this.toast = new DToastInstance(this, {});

      await render(
-        hbs`<DDefaultToast @data={{this.toast.options.data}} @autoClose={{false}} />`
+        hbs`<DDefaultToast @data={{this.toast.options.data}} @showProgressBar={{false}} @autoClose={{false}} />`
      );

      assert.dom(".fk-d-default-toast__progress-bar").doesNotExist();
--- a/app/assets/javascripts/discourse/tests/integration/components/float-kit/d-menu-test.js
+++ b/app/assets/javascripts/discourse/tests/integration/components/float-kit/d-menu-test.js
@ -43,6 +43,17 @@ module("Integration | Component | FloatKit | d-menu", function (hooks) {
    assert.dom(".fk-d-menu").hasText("content");
  });

+  test("@modalForMobile", async function (assert) {
+    this.site.mobileView = true;
+
+    await render(
+      hbs`<DMenu @inline={{true}} @modalForMobile={{true}} @content="content" />`
+    );
+    await open();
+
+    assert.dom(".fk-d-menu-modal").hasText("content");
+  });
+
  test("@onRegisterApi", async function (assert) {
    this.api = null;
    this.onRegisterApi = (api) => (this.api = api);
--- a/app/assets/javascripts/discourse/tests/unit/services/toasts-test.js
+++ b/app/assets/javascripts/discourse/tests/unit/services/toasts-test.js
@ -0,0 +1,21 @@
+import { getOwner } from "@ember/application";
+import { setupTest } from "ember-qunit";
+import { module, test } from "qunit";
+
+module("Unit | Service | Toasts", function (hooks) {
+  setupTest(hooks);
+
+  hooks.beforeEach(function () {
+    this.toasts = getOwner(this).lookup("service:toasts");
+  });
+
+  test("views option", async function (assert) {
+    this.toasts.show({ views: ["desktop"], data: { text: "foo" } });
+
+    assert.deepEqual(this.toasts.activeToasts.length, 1);
+
+    this.toasts.show({ views: ["mobile"], data: { text: "foo" } });
+
+    assert.ok(this.toasts.activeToasts.length < 2);
+  });
+});
--- a/app/assets/javascripts/float-kit/addon/components/d-default-toast.gjs
+++ b/app/assets/javascripts/float-kit/addon/components/d-default-toast.gjs
@ -13,7 +13,7 @@ const DDefaultToast = <template>
    }}
    ...attributes
  >
-    {{#if @autoClose}}
+    {{#if @showProgressBar}}
      <div
        class="fk-d-default-toast__progress-bar"
        {{didInsert @onRegisterProgressBar}}
--- a/app/assets/javascripts/float-kit/addon/components/d-menu.gjs
+++ b/app/assets/javascripts/float-kit/addon/components/d-menu.gjs
@ -5,14 +5,18 @@ import { concat } from "@ember/helper";
 import { action } from "@ember/object";
 import { inject as service } from "@ember/service";
 import { modifier } from "ember-modifier";
+import { and } from "truth-helpers";
 import DButton from "discourse/components/d-button";
+import DModal from "discourse/components/d-modal";
 import concatClass from "discourse/helpers/concat-class";
+import { isTesting } from "discourse-common/config/environment";
 import DFloatBody from "float-kit/components/d-float-body";
 import { MENU } from "float-kit/lib/constants";
 import DMenuInstance from "float-kit/lib/d-menu-instance";

 export default class DMenu extends Component {
  @service menu;
+  @service site;

  @tracked menuInstance = null;

@ -92,31 +96,56 @@ export default class DMenu extends Component {
    </DButton>

    {{#if this.menuInstance.expanded}}
-      <DFloatBody
-        @instance={{this.menuInstance}}
-        @trapTab={{this.options.trapTab}}
-        @mainClass={{concatClass
-          "fk-d-menu"
-          (concat this.options.identifier "-content")
-        }}
-        @innerClass="fk-d-menu__inner-content"
-        @role="dialog"
-        @inline={{this.options.inline}}
-        @portalOutletElement={{this.menu.portalOutletElement}}
-      >
-        {{#if (has-block)}}
-          {{yield this.componentArgs}}
-        {{else if (has-block "content")}}
-          {{yield this.componentArgs to="content"}}
-        {{else if this.options.component}}
-          <this.options.component
-            @data={{this.options.data}}
-            @close={{this.menuInstance.close}}
-          />
-        {{else if this.options.content}}
-          {{this.options.content}}
-        {{/if}}
-      </DFloatBody>
+      {{#if (and this.site.mobileView this.options.modalForMobile)}}
+        <DModal
+          @closeModal={{this.menuInstance.close}}
+          @hideHeader={{true}}
+          class={{concatClass
+            "fk-d-menu-modal"
+            (concat this.options.identifier "-content")
+          }}
+          @inline={{(isTesting)}}
+        >
+          {{#if (has-block)}}
+            {{yield this.componentArgs}}
+          {{else if (has-block "content")}}
+            {{yield this.componentArgs to="content"}}
+          {{else if this.options.component}}
+            <this.options.component
+              @data={{this.options.data}}
+              @close={{this.menuInstance.close}}
+            />
+          {{else if this.options.content}}
+            {{this.options.content}}
+          {{/if}}
+        </DModal>
+      {{else}}
+        <DFloatBody
+          @instance={{this.menuInstance}}
+          @trapTab={{this.options.trapTab}}
+          @mainClass={{concatClass
+            "fk-d-menu"
+            (concat this.options.identifier "-content")
+          }}
+          @innerClass="fk-d-menu__inner-content"
+          @role="dialog"
+          @inline={{this.options.inline}}
+          @portalOutletElement={{this.menu.portalOutletElement}}
+        >
+          {{#if (has-block)}}
+            {{yield this.componentArgs}}
+          {{else if (has-block "content")}}
+            {{yield this.componentArgs to="content"}}
+          {{else if this.options.component}}
+            <this.options.component
+              @data={{this.options.data}}
+              @close={{this.menuInstance.close}}
+            />
+          {{else if this.options.content}}
+            {{this.options.content}}
+          {{/if}}
+        </DFloatBody>
+      {{/if}}
    {{/if}}
  </template>
 }
--- a/app/assets/javascripts/float-kit/addon/components/d-toast.gjs
+++ b/app/assets/javascripts/float-kit/addon/components/d-toast.gjs
@ -4,6 +4,7 @@ import { registerDestructor } from "@ember/destroyable";
 import { action } from "@ember/object";
 import { cancel } from "@ember/runloop";
 import Modifier from "ember-modifier";
+import { and } from "truth-helpers";
 import concatClass from "discourse/helpers/concat-class";
 import discourseLater from "discourse-common/lib/later";
 import { bind } from "discourse-common/utils/decorators";
@ -127,7 +128,10 @@ export default class DToast extends Component {
      <@toast.options.component
        @data={{@toast.options.data}}
        @close={{@toast.close}}
-        @autoClose={{@toast.options.autoClose}}
+        @showProgressBar={{and
+          @toast.options.showProgressBar
+          @toast.options.autoClose
+        }}
        @onRegisterProgressBar={{this.registerProgressBar}}
      />
    </output>
--- a/app/assets/javascripts/float-kit/addon/lib/constants.js
+++ b/app/assets/javascripts/float-kit/addon/lib/constants.js
@ -68,6 +68,7 @@ export const MENU = {
    onClose: null,
    onShow: null,
    onRegisterApi: null,
+    modalForMobile: false,
  },
  portalOutletId: "d-menu-portal-outlet",
 };
@ -79,5 +80,7 @@ export const TOAST = {
    autoClose: true,
    duration: 3000,
    component: DDefaultToast,
+    showProgressBar: true,
+    views: ["desktop", "mobile"],
  },
 };
--- a/app/assets/javascripts/float-kit/addon/lib/d-toast-instance.js
+++ b/app/assets/javascripts/float-kit/addon/lib/d-toast-instance.js
@ -5,6 +5,7 @@ import uniqueId from "discourse/helpers/unique-id";
 import { TOAST } from "float-kit/lib/constants";

 export default class DToastInstance {
+  @service site;
  @service toasts;

  options = null;
@ -19,4 +20,8 @@ export default class DToastInstance {
  close() {
    this.toasts.close(this);
  }
+
+  get isValidForView() {
+    return this.options.views.includes(this.site.desktopView ? "desktop" : "mobile");
+  }
 }
--- a/app/assets/javascripts/float-kit/addon/services/toasts.js
+++ b/app/assets/javascripts/float-kit/addon/services/toasts.js
@ -23,8 +23,15 @@ export default class Toasts extends Service {
   */
  @action
  show(options = {}) {
-    const instance = new DToastInstance(getOwner(this), options);
-    this.activeToasts.push(instance);
+    const instance = new DToastInstance(getOwner(this), {
+      component: DDefaultToast,
+      ...options,
+    });
+
+    if (instance.isValidForView) {
+      this.activeToasts.push(instance);
+    }
+
    return instance;
  }

@ -39,7 +46,7 @@ export default class Toasts extends Service {
  default(options = {}) {
    options.data.theme = "default";

-    return this.show({ ...options, component: DDefaultToast });
+    return this.show(options);
  }

  /**
@ -52,9 +59,9 @@ export default class Toasts extends Service {
  @action
  success(options = {}) {
    options.data.theme = "success";
-    options.data.icon = "check";
+    options.data.icon ??= "check";

-    return this.show({ ...options, component: DDefaultToast });
+    return this.show(options);
  }

  /**
@ -67,9 +74,9 @@ export default class Toasts extends Service {
  @action
  error(options = {}) {
    options.data.theme = "error";
-    options.data.icon = "exclamation-triangle";
+    options.data.icon ??= "exclamation-triangle";

-    return this.show({ ...options, component: DDefaultToast });
+    return this.show(options);
  }

  /**
@ -82,9 +89,9 @@ export default class Toasts extends Service {
  @action
  warning(options = {}) {
    options.data.theme = "warning";
-    options.data.icon = "exclamation-circle";
+    options.data.icon ??= "exclamation-circle";

-    return this.show({ ...options, component: DDefaultToast });
+    return this.show(options);
  }

  /**
@ -97,9 +104,9 @@ export default class Toasts extends Service {
  @action
  info(options = {}) {
    options.data.theme = "info";
-    options.data.icon = "info-circle";
+    options.data.icon ??= "info-circle";

-    return this.show({ ...options, component: DDefaultToast });
+    return this.show(options);
  }

  /**
--- a/app/assets/stylesheets/common/components/bookmark-menu.scss
+++ b/app/assets/stylesheets/common/components/bookmark-menu.scss
@ -1,91 +1,73 @@
 .bookmark-menu-content {
  .bookmark-menu__body {
    background: var(--secondary);
-    list-style: none;
    display: flex;
    flex-direction: column;
-    color: var(--primary);
+    min-width: 200px;
+  }
+  .bookmark-menu__actions {
+    margin: 0;
+    padding: 0;
+    list-style: none;
+  }
+  .bookmark-menu__title {
+    display: flex;
+    align-items: center;
+    gap: 0.75em;
+    background: var(--tertiary-low);
+    color: var(--tertiary);
+    padding: 0.75rem;
+    font-weight: bold;

-    .bookmark-menu__actions {
-      margin: 0;
-      padding: 0;
-      list-style: none;
+    .d-icon {
+      color: var(--tertiary);
    }
  }

-  .bookmark-menu {
-    &__text {
-      display: flex;
-      align-items: left;
-    }
+  .bookmark-menu__row {
+    width: 100%;
+    display: flex;

-    &__row {
-      border-bottom: 1px solid var(--primary-low);
-      width: 100%;
-      display: flex;
-      align-items: left;
+    &:hover,
+    &:focus {
+      background: var(--tertiary-very-low);

-      &:hover,
-      &:focus {
-        background: var(--tertiary-very-low);
-      }
-
-      &-title {
-        font-size: var(--font-down-1);
-        padding: 0.75rem;
-        border-bottom: 1px solid var(--primary-low);
-      }
-
-      &-btn {
-        margin: 0;
-        padding: 0.75rem;
-        width: 100%;
-        text-align: left;
-        justify-content: left;
-
-        .d-icon {
-          color: var(--primary);
-        }
-
-        .d-button-label {
-          color: var(--primary);
-          font-size: var(--font-down-1);
-        }
-        &:hover,
-        &:focus {
-          background: var(--tertiary-very-low);
-        }
-      }
-
-      &.-edit {
-        .d-icon {
-          margin-right: 5px;
-        }
-      }
-
-      &.-remove {
-        .d-icon {
-          color: var(--danger);
-        }
-
-        &:hover,
-        &:focus {
-          background: var(--danger-low);
-        }
-      }
-
-      &:last-child {
-        border-bottom: none;
-      }
-
-      &.-no-reminder {
-        border-bottom: 2px solid var(--primary-low);
+      &.--remove {
+        background: var(--danger-low);
      }
    }
  }

  .bookmark-menu__row-title {
-    font-weight: 900;
    padding: 0.75rem;
+    border-bottom: 1px solid var(--primary-low);
+    font-weight: bold;
+  }
+
+  .bookmark-menu__row-btn {
+    margin: 0;
+    padding: 0.75rem !important;
+    width: 100%;
+    text-align: left;
+    justify-content: left !important;
+    gap: 0.75em;
+    color: var(--primary);
+    &:hover,
+    &:focus {
+      color: var(--primary) !important;
+      background: var(--tertiary-very-low);
+    }
+    .--remove & {
+      color: var(--danger);
+    }
+
+    .d-button-label {
+      color: inherit;
+    }
+
+    .d-icon {
+      color: inherit;
+      margin: 0 !important;
+    }
  }
 }
--- a/app/assets/stylesheets/common/float-kit/d-default-toast.scss
+++ b/app/assets/stylesheets/common/float-kit/d-default-toast.scss
@ -110,7 +110,6 @@

  &__message {
    display: flex;
-    margin-top: 0.5rem;
    color: var(--primary-high);
  }
 }
--- a/app/assets/stylesheets/common/float-kit/d-menu.scss
+++ b/app/assets/stylesheets/common/float-kit/d-menu.scss
@ -56,7 +56,6 @@
  }

  .arrow {
-    z-index: z("max");
    position: absolute;
    color: var(--secondary);
  }
@ -70,7 +69,7 @@

  &[data-placement^="bottom"] {
    .arrow {
-      top: -9px;
+      top: -10px;
    }
  }

--- a/app/assets/stylesheets/mobile.scss
+++ b/app/assets/stylesheets/mobile.scss
@ -6,3 +6,5 @@
@import "mobile/components/_index";

@import "mobile/select-kit/_index";
+
+@import "mobile/float-kit/_index";
--- a/app/assets/stylesheets/mobile/components/_index.scss
+++ b/app/assets/stylesheets/mobile/components/_index.scss
@ -2,3 +2,4 @@
@import "user-card";
@import "user-stream-item";
@import "more-topics";
+@import "bookmark-menu";
--- a/app/assets/stylesheets/mobile/components/bookmark-menu.scss
+++ b/app/assets/stylesheets/mobile/components/bookmark-menu.scss
@ -0,0 +1,11 @@
+.bookmark-menu {
+  &__row {
+    border-bottom: 1px solid var(--primary-low);
+    &:last-child {
+      border-bottom: none;
+    }
+  }
+  &__row-title {
+    padding: 0.75rem;
+  }
+}
--- a/app/assets/stylesheets/mobile/float-kit/_index.scss
+++ b/app/assets/stylesheets/mobile/float-kit/_index.scss
@ -0,0 +1 @@
+@import "d-menu";
--- a/app/assets/stylesheets/mobile/float-kit/d-menu.scss
+++ b/app/assets/stylesheets/mobile/float-kit/d-menu.scss
@ -0,0 +1,5 @@
+.fk-d-menu-modal {
+  .d-modal__body {
+    padding: 0;
+  }
+}
--- a/spec/system/bookmarks_spec.rb
+++ b/spec/system/bookmarks_spec.rb
@ -40,7 +40,8 @@ describe "Bookmarking posts and topics", type: :system do
    expect(page).to have_content(I18n.t("js.bookmarks.bookmarked_success"))

    bookmark_menu.click_menu_option("tomorrow")
-    expect(page).to have_content(I18n.t("js.bookmarks.reminder_set_success"))
+
+    expect(page).to have_no_css(".bookmark-menu-content")
    expect(Bookmark.find_by(bookmarkable: post, user: current_user).reminder_at).not_to be_blank
  end