DEV: Refactor `GroupNotificationsButton` into `userPrivateMessages.group` route (#21930)

Why this change?

Before this change, the `GroupNotificationsButton` is rendered in the
template of `userPrivateMessages` route based on a conditional that
checks if the `isGroup` property is true. However, the `isGroup`
property is determined based on the child route that is rendered.
However, this leads to "jankiness" in the UI because the
`GroupNotificationsButton` will be rendered once the route is entered
even if the model for the child route has not been resolved yet.

What is the solution?

In order to avoid this, we move the rendering of the
`GroupNotificationsButton` into the template of the
`userPrivateMessages.group` route and rely on the `in-element` helper to
render it into the right spot in the template of the
`userPrivateMessages` route.
This commit is contained in:
Alan Guo Xiang Tan 2023-06-07 07:22:50 +09:00 committed by GitHub
parent 81645a3082
commit 1cbc65ba79
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 33 additions and 23 deletions

View File

@ -1,6 +1,6 @@
import I18n from "I18n"; import I18n from "I18n";
import Controller, { inject as controller } from "@ember/controller"; import Controller, { inject as controller } from "@ember/controller";
import { computed } from "@ember/object"; import { action, computed } from "@ember/object";
export default class extends Controller { export default class extends Controller {
@controller user; @controller user;
@ -25,10 +25,14 @@ export default class extends Controller {
return this.#linkText("unread"); return this.#linkText("unread");
} }
get navigationControlsButton() {
return document.getElementById("navigation-controls__button");
}
#linkText(type) { #linkText(type) {
const count = this.pmTopicTrackingState?.lookupCount(type, { const count = this.pmTopicTrackingState?.lookupCount(type, {
inboxFilter: "group", inboxFilter: "group",
groupName: this.groupName, groupName: this.group.name,
}); });
if (count === 0) { if (count === 0) {
@ -37,4 +41,9 @@ export default class extends Controller {
return I18n.t(`user.messages.${type}_with_count`, { count }); return I18n.t(`user.messages.${type}_with_count`, { count });
} }
} }
@action
changeGroupNotificationLevel(notificationLevel) {
this.group.setNotification(notificationLevel, this.get("user.model.id"));
}
} }

View File

@ -97,11 +97,6 @@ export default class extends Controller {
return content; return content;
} }
@action
changeGroupNotificationLevel(notificationLevel) {
this.group.setNotification(notificationLevel, this.get("user.model.id"));
}
@action @action
onMessagesDropdownChange(item) { onMessagesDropdownChange(item) {
return DiscourseURL.routeTo(item); return DiscourseURL.routeTo(item);

View File

@ -23,7 +23,7 @@ export default (inboxType, filter) => {
model() { model() {
const username = this.modelFor("user").get("username_lower"); const username = this.modelFor("user").get("username_lower");
const groupName = this.modelFor("userPrivateMessages.group"); const groupName = this.modelFor("userPrivateMessages.group").name;
let topicListFilter = `topics/private-messages-group/${username}/${groupName}`; let topicListFilter = `topics/private-messages-group/${username}/${groupName}`;
@ -60,9 +60,7 @@ export default (inboxType, filter) => {
groupName = filters.pop(); groupName = filters.pop();
} }
const group = this.modelFor("user") const group = this.modelFor("userPrivateMessages.group");
.get("groups")
.filterBy("name", groupName)[0];
this.setProperties({ groupName, group }); this.setProperties({ groupName, group });
}, },

View File

@ -2,10 +2,12 @@ import DiscourseRoute from "discourse/routes/discourse";
export default class extends DiscourseRoute { export default class extends DiscourseRoute {
model(params) { model(params) {
return params.name; return this.modelFor("user")
.get("groups")
.filterBy("name", params.name.toLowerCase())[0];
} }
setupController(controller, model) { setupController(controller, model) {
controller.set("groupName", model); controller.set("group", model);
} }
} }

View File

@ -3,7 +3,7 @@
<DNavigationItem <DNavigationItem
@route="userPrivateMessages.group.index" @route="userPrivateMessages.group.index"
@class="user-nav__messages-group-latest" @class="user-nav__messages-group-latest"
@model={{this.groupName}} @model={{this.group.name}}
@ariaCurrentContext="subNav" @ariaCurrentContext="subNav"
> >
{{d-icon "envelope"}} {{d-icon "envelope"}}
@ -13,7 +13,7 @@
{{#if this.viewingSelf}} {{#if this.viewingSelf}}
<DNavigationItem <DNavigationItem
@route="userPrivateMessages.group.new" @route="userPrivateMessages.group.new"
@model={{this.groupName}} @model={{this.group.name}}
@class="user-nav__messages-group-new" @class="user-nav__messages-group-new"
@ariaCurrentContext="subNav" @ariaCurrentContext="subNav"
> >
@ -23,7 +23,7 @@
<DNavigationItem <DNavigationItem
@route="userPrivateMessages.group.unread" @route="userPrivateMessages.group.unread"
@model={{this.groupName}} @model={{this.group.name}}
@class="user-nav__messages-group-unread" @class="user-nav__messages-group-unread"
@ariaCurrentContext="subNav" @ariaCurrentContext="subNav"
> >
@ -34,7 +34,7 @@
<DNavigationItem <DNavigationItem
@route="userPrivateMessages.group.archive" @route="userPrivateMessages.group.archive"
@class="user-nav__messages-group-archive" @class="user-nav__messages-group-archive"
@model={{this.groupName}} @model={{this.group.name}}
@ariaCurrentContext="subNav" @ariaCurrentContext="subNav"
> >
{{d-icon "archive"}} {{d-icon "archive"}}
@ -43,4 +43,11 @@
{{/if}} {{/if}}
</UserNav::MessagesSecondaryNav> </UserNav::MessagesSecondaryNav>
{{#in-element this.navigationControlsButton}}
<GroupNotificationsButton
@value={{this.group.group_user.notification_level}}
@onChange={{this.changeGroupNotificationLevel}}
/>
{{/in-element}}
{{outlet}} {{outlet}}

View File

@ -27,12 +27,7 @@
{{/if}} {{/if}}
{{/if}} {{/if}}
{{#if this.isGroup}} <span id="navigation-controls__button"></span>
<GroupNotificationsButton
@value={{this.group.group_user.notification_level}}
@onChange={{this.changeGroupNotificationLevel}}
/>
{{/if}}
{{#if this.showNewPM}} {{#if this.showNewPM}}
<DButton <DButton

View File

@ -131,6 +131,10 @@
@include breakpoint(extra-large) { @include breakpoint(extra-large) {
font-size: var(--font-down-1); font-size: var(--font-down-1);
} }
span {
display: inline-flex;
}
} }
.nav-pills { .nav-pills {