FIX: Keep track of suggestion updates during scrolling and navigation. (#23190)

This PR changes how we track which lists are available for a topic and how we decide which is the active one. The new approach centralizes everything in the service, and exposes functions for adding/removing a list, which each calls via `did-insert/will-destroy` modifiers.

It makes it much easier to track and update state when navigated to another topic or PM, ensuring things get updated correctly.
This commit is contained in:
Roman Rizzi 2023-08-22 14:10:15 -03:00 committed by GitHub
parent a3410e7dc3
commit 7d0e2b9e7b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 96 additions and 69 deletions

View File

@ -1,14 +1,15 @@
<div class="more-topics__container" {{did-insert this.buildListPills}}> <div class="more-topics__container">
{{#unless this.singleList}} {{#unless this.singleList}}
<div class="row"> <div class="row">
<ul class="nav nav-pills"> <ul class="nav nav-pills">
{{#each this.availablePills as |pill|}} {{#each this.availableTabs as |tab|}}
<li> <li>
<DButton <DButton
@translatedTitle={{pill.name}} @translatedTitle={{tab.name}}
@translatedLabel={{pill.name}} @translatedLabel={{tab.name}}
@class={{if pill.selected "active"}} @class={{if (eq tab.id this.selectedTab) "active"}}
@action={{action "rememberTopicListPreference" pill.id}} @action={{action "rememberTopicListPreference" tab.id}}
@icon={{tab.icon}}
/> />
</li> </li>
{{/each}} {{/each}}

View File

@ -1,8 +1,6 @@
import Component from "@glimmer/component"; import Component from "@glimmer/component";
import { inject as service } from "@ember/service"; import { inject as service } from "@ember/service";
import { next } from "@ember/runloop";
import { action, computed } from "@ember/object"; import { action, computed } from "@ember/object";
import { tracked } from "@glimmer/tracking";
import I18n from "I18n"; import I18n from "I18n";
import { categoryBadgeHTML } from "discourse/helpers/category-link"; import { categoryBadgeHTML } from "discourse/helpers/category-link";
import getURL from "discourse-common/lib/get-url"; import getURL from "discourse-common/lib/get-url";
@ -15,59 +13,24 @@ export default class MoreTopics extends Component {
@service topicTrackingState; @service topicTrackingState;
@service currentUser; @service currentUser;
@tracked availablePills = [];
@tracked singleList = false;
@action @action
rememberTopicListPreference(value) { rememberTopicListPreference(value) {
// Don't change the preference when selecting related PMs. this.moreTopicsPreferenceTracking.updatePreference(value);
// It messes with the topics pref.
const rememberPref = value !== "related-messages";
this.moreTopicsPreferenceTracking.updatePreference(value, rememberPref);
this.buildListPills();
} }
@action @computed("moreTopicsPreferenceTracking.topicLists")
buildListPills() { get singleList() {
next(() => { return this.availableTabs.length === 1;
const pills = Array.from( }
document.querySelectorAll(".more-topics__list")
).map((topicList) => {
return {
name: topicList.dataset.mobileTitle,
id: topicList.dataset.listId,
};
});
if (pills.length === 0) { @computed("moreTopicsPreferenceTracking.selectedTab")
return; get selectedTab() {
} else if (pills.length === 1) { return this.moreTopicsPreferenceTracking.selectedTab;
this.singleList = true; }
}
let preference = this.moreTopicsPreferenceTracking.preference; @computed("moreTopicsPreferenceTracking.topicLists")
// Scenario where we have a preference, but there get availableTabs() {
// are no more elements in it. return this.moreTopicsPreferenceTracking.topicLists;
const listPresent = pills.find((pill) => pill.id === preference);
if (!listPresent) {
const rememberPref = false;
this.moreTopicsPreferenceTracking.updatePreference(
pills[0].id,
rememberPref
);
preference = pills[0].id;
}
pills.forEach((pill) => {
pill.selected = pill.id === preference;
});
this.availablePills = pills;
});
} }
@computed( @computed(

View File

@ -3,8 +3,8 @@
class="more-topics__list {{if this.hidden 'hidden'}}" class="more-topics__list {{if this.hidden 'hidden'}}"
role="complementary" role="complementary"
aria-labelledby="related-messages-title" aria-labelledby="related-messages-title"
data-mobile-title={{i18n "related_messages.pill"}} {{did-insert this.registerList}}
data-list-id={{this.listId}} {{will-destroy this.removeList}}
> >
<h3 id="related-messages-title" class="more-topics__list-title"> <h3 id="related-messages-title" class="more-topics__list-title">
{{i18n "related_messages.title"}} {{i18n "related_messages.title"}}

View File

@ -1,7 +1,8 @@
import Component from "@glimmer/component"; import Component from "@glimmer/component";
import { computed } from "@ember/object"; import { action, computed } from "@ember/object";
import getURL from "discourse-common/lib/get-url"; import getURL from "discourse-common/lib/get-url";
import { inject as service } from "@ember/service"; import { inject as service } from "@ember/service";
import I18n from "I18n";
export default class RelatedMessages extends Component { export default class RelatedMessages extends Component {
@service moreTopicsPreferenceTracking; @service moreTopicsPreferenceTracking;
@ -9,9 +10,22 @@ export default class RelatedMessages extends Component {
listId = "related-Messages"; listId = "related-Messages";
@computed("moreTopicsPreferenceTracking.preference") @computed("moreTopicsPreferenceTracking.selectedTab")
get hidden() { get hidden() {
return this.moreTopicsPreferenceTracking.preference !== this.listId; return this.moreTopicsPreferenceTracking.selectedTab !== this.listId;
}
@action
registerList() {
this.moreTopicsPreferenceTracking.registerTopicList({
name: I18n.t("related_messages.pill"),
id: this.listId,
});
}
@action
removeList() {
this.moreTopicsPreferenceTracking.removeTopicList(this.listId);
} }
get targetUser() { get targetUser() {

View File

@ -3,8 +3,8 @@
class="more-topics__list {{if this.hidden 'hidden'}}" class="more-topics__list {{if this.hidden 'hidden'}}"
role="complementary" role="complementary"
aria-labelledby="suggested-topics-title" aria-labelledby="suggested-topics-title"
data-mobile-title={{i18n "suggested_topics.pill"}} {{did-insert this.registerList}}
data-list-id={{this.listId}} {{will-destroy this.removeList}}
> >
<UserTip @id="suggested_topics" @selector=".user-tip-reference" /> <UserTip @id="suggested_topics" @selector=".user-tip-reference" />

View File

@ -1,6 +1,7 @@
import Component from "@glimmer/component"; import Component from "@glimmer/component";
import { computed } from "@ember/object"; import { action, computed } from "@ember/object";
import { inject as service } from "@ember/service"; import { inject as service } from "@ember/service";
import I18n from "I18n";
export default class SuggestedTopics extends Component { export default class SuggestedTopics extends Component {
@service moreTopicsPreferenceTracking; @service moreTopicsPreferenceTracking;
@ -17,8 +18,21 @@ export default class SuggestedTopics extends Component {
} }
} }
@computed("moreTopicsPreferenceTracking.preference") @computed("moreTopicsPreferenceTracking.selectedTab")
get hidden() { get hidden() {
return this.moreTopicsPreferenceTracking.preference !== this.listId; return this.moreTopicsPreferenceTracking.selectedTab !== this.listId;
}
@action
registerList() {
this.moreTopicsPreferenceTracking.registerTopicList({
name: I18n.t("suggested_topics.pill"),
id: this.listId,
});
}
@action
removeList() {
this.moreTopicsPreferenceTracking.removeTopicList(this.listId);
} }
} }

View File

@ -6,18 +6,49 @@ const TOPIC_LIST_PREFERENCE_KEY = "more-topics-list-preference";
export default class MoreTopicsPreferenceTracking extends Service { export default class MoreTopicsPreferenceTracking extends Service {
@service keyValueStore; @service keyValueStore;
@tracked preference; @tracked selectedTab = null;
@tracked topicLists = [];
memoryTab = null;
init() { init() {
super.init(...arguments); super.init(...arguments);
this.preference = this.keyValueStore.get(TOPIC_LIST_PREFERENCE_KEY); this.memoryTab = this.keyValueStore.get(TOPIC_LIST_PREFERENCE_KEY);
} }
updatePreference(value, rememberPref = true) { updatePreference(value) {
// Don't change the preference when selecting related PMs.
// It messes with the topics pref.
const rememberPref = value !== "related-messages";
if (rememberPref) { if (rememberPref) {
this.keyValueStore.set({ key: TOPIC_LIST_PREFERENCE_KEY, value }); this.keyValueStore.set({ key: TOPIC_LIST_PREFERENCE_KEY, value });
this.memoryTab = value;
} }
this.preference = value; this.selectedTab = value;
}
registerTopicList(item) {
// We have a preference stored and the list exists.
if (this.memoryTab && this.memoryTab === item.id) {
this.selectedTab = item.id;
}
// Use the first list as a default. Future lists may override this
// if they match the stored preference.
if (!this.selectedTab) {
this.selectedTab = item.id;
}
this.topicLists = [...this.topicLists, item];
}
removeTopicList(itemId) {
this.topicLists = this.topicLists.filter((item) => item.id !== itemId);
if (this.selectedTab === itemId) {
this.selectedTab = this.topicLists[0]?.id;
}
} }
} }

View File

@ -9,6 +9,10 @@
.btn { .btn {
padding: 0.5em 0.65em; padding: 0.5em 0.65em;
} }
.btn.active .d-icon {
color: var(--primary-low);
}
} }
} }