From a2fcc360dddbda71a3f231c1670b12f790948a5a Mon Sep 17 00:00:00 2001 From: jbrw Date: Wed, 15 Dec 2021 13:47:31 -0500 Subject: [PATCH] UX: Show group card with animated loading state (#15253) * Remove _calculateTopOffset entirely * Show group card with animated loading state Showing the animated loading state before rending the actual content prevents an awkward scroll position jump when displaying this card. This mimics the behaviour of the user card (which uses the same `CardContentsBase` mixin). * Fix two user card issues 1. A JS console error (with no consequences) when clicking a group mention 2. User cards weren't being loaded from the header (for example, for PMs) Co-authored-by: Penar Musaraj --- .../app/components/group-card-contents.js | 8 +- .../app/mixins/card-contents-base.js | 21 +-- .../components/group-card-contents.hbs | 128 ++++++++++-------- .../app/widgets/header-topic-info.js | 3 +- 4 files changed, 82 insertions(+), 78 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/group-card-contents.js b/app/assets/javascripts/discourse/app/components/group-card-contents.js index dc16eef64c3..cb073a6733d 100644 --- a/app/assets/javascripts/discourse/app/components/group-card-contents.js +++ b/app/assets/javascripts/discourse/app/components/group-card-contents.js @@ -41,11 +41,13 @@ export default Component.extend(CardContentsBase, CleansUp, { }, _showCallback(username, $target) { - this.store + this._positionCard($target); + this.setProperties({ visible: true, loading: true }); + + return this.store .find("group", username) .then((group) => { - this.setProperties({ group, visible: true }); - this._positionCard($target); + this.setProperties({ group }); if (!group.flair_url && !group.flair_bg_color) { group.set("flair_url", "fa-users"); } diff --git a/app/assets/javascripts/discourse/app/mixins/card-contents-base.js b/app/assets/javascripts/discourse/app/mixins/card-contents-base.js index b56179c7340..e23b9370ae0 100644 --- a/app/assets/javascripts/discourse/app/mixins/card-contents-base.js +++ b/app/assets/javascripts/discourse/app/mixins/card-contents-base.js @@ -4,7 +4,6 @@ import DiscourseURL from "discourse/lib/url"; import Mixin from "@ember/object/mixin"; import afterTransition from "discourse/lib/after-transition"; import { escapeExpression } from "discourse/lib/utilities"; -import headerOutletHeights from "discourse/lib/header-outlet-height"; import { inject as service } from "@ember/service"; import { wantsNewWindow } from "discourse/lib/intercept-click"; import { bind } from "discourse-common/utils/decorators"; @@ -63,7 +62,7 @@ export default Mixin.create({ } const closestArticle = target.closest("article"); - const postId = closestArticle ? closestArticle.dataset.postId : null; + const postId = closestArticle?.dataset?.postId || null; const wasVisible = this.visible; const previousTarget = this.cardTarget; @@ -164,9 +163,9 @@ export default Mixin.create({ return false; }, - _topicHeaderTrigger(username, $target) { + _topicHeaderTrigger(username, target) { this.setProperties({ isFixed: true, isDocked: true }); - return this._show(username, $target); + return this._show(username, target); }, _bindMobileScroll() { @@ -234,10 +233,9 @@ export default Mixin.create({ } } - position.top -= this._calculateTopOffset( - $("#main-outlet").offset(), - headerOutletHeights() - ); + // It looks better to have the card aligned slightly higher + position.top -= 24; + if (isFixed) { position.top -= $("html").scrollTop(); //if content is fixed and will be cut off on the bottom, display it above... @@ -286,13 +284,6 @@ export default Mixin.create({ }); }, - // some plugins/themes modify the page layout and may - // need to override this calculation for the card to - // position correctly - _calculateTopOffset(mainOutletOffset, outletHeights) { - return mainOutletOffset.top - outletHeights; - }, - @bind _hide() { if (!this.visible) { diff --git a/app/assets/javascripts/discourse/app/templates/components/group-card-contents.hbs b/app/assets/javascripts/discourse/app/templates/components/group-card-contents.hbs index 3527dac61e4..ef5908bf88e 100644 --- a/app/assets/javascripts/discourse/app/templates/components/group-card-contents.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/group-card-contents.hbs @@ -1,69 +1,81 @@ {{#if visible}}
-
-
- - {{avatar-flair - flairName=group.name - flairUrl=group.flair_url - flairBgColor=group.flair_bg_color - flairColor=group.flair_color - }} - + {{#if this.loading}} +
+
+
+
-
- -

- {{group.name}} -

- {{#if group.full_name}} -

{{group.full_name}}

- {{else}} -

{{group.name}}

- {{/if}} - -
-
    -
  • - {{group-membership-button - model=group - showLogin=(route-action "showLogin") - }} -
    • - {{#if group.messageable}} -
  • - {{d-button - action=(action "messageGroup") - class="btn-primary group-message-button inline" - icon="envelope" - label="groups.message" - }} -
    • - {{/if}} -
-
- {{#if this.group.bio_excerpt}}
-
- {{html-safe this.group.bio_excerpt}} -
+
- {{/if}} - - {{#if group.members}} -
-
- {{#each group.members as |user|}} - {{bound-avatar user "tiny"}} - {{/each}} - {{#if showMoreMembers}} - - +{{moreMembersCount}} {{i18n "more"}} - + {{else}} +
+
+ + {{avatar-flair + flairName=group.name + flairUrl=group.flair_url + flairBgColor=group.flair_bg_color + flairColor=group.flair_color + }} + +
+
+ +

+ {{group.name}} +

+ {{#if group.full_name}} +

{{group.full_name}}

+ {{else}} +

{{group.name}}

+ {{/if}} + +
+
    +
  • + {{group-membership-button + model=group + showLogin=(route-action "showLogin") + }} +
    • + {{#if group.messageable}} +
  • + {{d-button + action=(action "messageGroup") + class="btn-primary group-message-button inline" + icon="envelope" + label="groups.message" + }} +
    • {{/if}} -
+
+ + {{#if this.group.bio_excerpt}} +
+
+ {{html-safe this.group.bio_excerpt}} +
+
+ {{/if}} + + {{#if group.members}} +
+
+ {{#each group.members as |user|}} + {{bound-avatar user "tiny"}} + {{/each}} + {{#if showMoreMembers}} + + +{{moreMembersCount}} {{i18n "more"}} + + {{/if}} +
+
+ {{/if}} {{/if}}
{{/if}} diff --git a/app/assets/javascripts/discourse/app/widgets/header-topic-info.js b/app/assets/javascripts/discourse/app/widgets/header-topic-info.js index b3264a03bb8..80e19243b7e 100644 --- a/app/assets/javascripts/discourse/app/widgets/header-topic-info.js +++ b/app/assets/javascripts/discourse/app/widgets/header-topic-info.js @@ -46,11 +46,10 @@ createWidget("topic-header-participant", { }, click(e) { - const $target = $(e.target); this.appEvents.trigger( `topic-header:trigger-${this.attrs.type}-card`, this.attrs.username, - $target + e.target ); e.preventDefault(); },