imterah/element-portable
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Clean up TODO comments for new room list

Browse source 
All relevant TODOs should still be present, and reference an issue for easy finding.
This commit is contained in:
Travis Ralston 2020-06-28 20:03:04 -06:00
parent b9ce10bd6d
commit ee2c216c4d
25 changed files with 117 additions and 93 deletions
Download patch file Download diff file Expand all files Collapse all files

3
src/stores/room-list/ListLayout.ts
View file

@ -85,7 +85,8 @@ export class ListLayout {
}
public get defaultVisibleTiles(): number {
// TODO: Remove dogfood flag
// TODO: Remove dogfood flag: https://github.com/vector-im/riot-web/issues/14231
// TODO: Resolve dogfooding: https://github.com/vector-im/riot-web/issues/14137
const val = Number(localStorage.getItem("mx_dogfood_rl_defTiles") || 4);
return val + RESIZER_BOX_FACTOR;
}

2
src/stores/room-list/MessagePreviewStore.ts
View file

@ -192,7 +192,7 @@ export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
protected async onAction(payload: ActionPayload) {
if (!this.matrixClient) return;
// TODO: Remove when new room list is made the default
// TODO: Remove when new room list is made the default: https://github.com/vector-im/riot-web/issues/14231
if (!RoomListStoreTempProxy.isUsingNewStore()) return;
if (payload.action === 'MatrixActions.Room.timeline' || payload.action === 'MatrixActions.Event.decrypted') {

31
src/stores/room-list/RoomListStore2.ts
View file

@ -72,7 +72,7 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
return this._matrixClient;
}
// TODO: Remove enabled flag when the old RoomListStore goes away
// TODO: Remove enabled flag with the old RoomListStore: https://github.com/vector-im/riot-web/issues/14231
private checkEnabled() {
this.enabled = SettingsStore.isFeatureEnabled("feature_new_room_list");
if (this.enabled) {
@ -89,7 +89,7 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
}
private onRVSUpdate = () => {
if (!this.enabled) return; // TODO: Remove enabled flag when RoomListStore2 takes over
if (!this.enabled) return; // TODO: Remove with https://github.com/vector-im/riot-web/issues/14231
if (!this.matrixClient) return; // We assume there won't be RVS updates without a client
const activeRoomId = RoomViewStore.getRoomId();
@ -99,6 +99,7 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
const activeRoom = this.matrixClient.getRoom(activeRoomId);
if (!activeRoom) throw new Error(`${activeRoomId} is current in RVS but missing from client`);
if (activeRoom !== this.algorithm.stickyRoom) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`Changing sticky room to ${activeRoomId}`);
this.algorithm.stickyRoom = activeRoom;
}
@ -112,7 +113,7 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
return;
}
// TODO: Remove this once the RoomListStore becomes default
// TODO: Remove with https://github.com/vector-im/riot-web/issues/14231
this.checkEnabled();
if (!this.enabled) return;
@ -163,12 +164,14 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
console.warn(`Own read receipt was in unknown room ${room.roomId}`);
return;
}
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Got own read receipt in ${room.roomId}`);
await this.handleRoomUpdate(room, RoomUpdateCause.ReadReceipt);
return;
}
} else if (payload.action === 'MatrixActions.Room.tags') {
const roomPayload = (<any>payload); // TODO: Type out the dispatcher types
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Got tag change in ${roomPayload.room.roomId}`);
await this.handleRoomUpdate(roomPayload.room, RoomUpdateCause.PossibleTagChange);
} else if (payload.action === 'MatrixActions.Room.timeline') {
@ -180,10 +183,12 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
const roomId = eventPayload.event.getRoomId();
const room = this.matrixClient.getRoom(roomId);
const tryUpdate = async (updatedRoom: Room) => {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Live timeline event ${eventPayload.event.getId()} in ${updatedRoom.roomId}`);
if (eventPayload.event.getType() === 'm.room.tombstone' && eventPayload.event.getStateKey() === '') {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Got tombstone event - regenerating room list`);
// TODO: We could probably be smarter about this
// TODO: We could probably be smarter about this: https://github.com/vector-im/riot-web/issues/14035
await this.regenerateAllLists();
return; // don't pass the update down - we will have already handled it in the regen
}
@ -208,13 +213,15 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
console.warn(`Event ${eventPayload.event.getId()} was decrypted in an unknown room ${roomId}`);
return;
}
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Decrypted timeline event ${eventPayload.event.getId()} in ${roomId}`);
// TODO: Check that e2e rooms are calculated correctly on initial load.
// TODO: Verify that e2e rooms are handled on init: https://github.com/vector-im/riot-web/issues/14238
// It seems like when viewing the room the timeline is decrypted, rather than at startup. This could
// cause inaccuracies with the list ordering. We may have to decrypt the last N messages of every room :(
await this.handleRoomUpdate(room, RoomUpdateCause.Timeline);
} else if (payload.action === 'MatrixActions.accountData' && payload.event_type === 'm.direct') {
const eventPayload = (<any>payload); // TODO: Type out the dispatcher types
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Received updated DM map`);
const dmMap = eventPayload.event.getContent();
for (const userId of Object.keys(dmMap)) {
@ -236,6 +243,7 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
} else if (payload.action === 'MatrixActions.Room.myMembership') {
const membershipPayload = (<any>payload); // TODO: Type out the dispatcher types
if (membershipPayload.oldMembership !== "join" && membershipPayload.membership === "join") {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Handling new room ${membershipPayload.room.roomId}`);
await this.algorithm.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom);
return;
@ -243,6 +251,7 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
// If it's not a join, it's transitioning into a different list (possibly historical)
if (membershipPayload.oldMembership !== membershipPayload.membership) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Handling membership change in ${membershipPayload.room.roomId}`);
await this.algorithm.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.PossibleTagChange);
return;
@ -253,6 +262,7 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
private async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<any> {
const shouldUpdate = await this.algorithm.handleRoomUpdate(room, cause);
if (shouldUpdate) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[DEBUG] Room "${room.name}" (${room.roomId}) triggered by ${cause} requires list update`);
this.emit(LISTS_UPDATE_EVENT, this);
}
@ -260,6 +270,7 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
public async setTagSorting(tagId: TagID, sort: SortAlgorithm) {
await this.algorithm.setTagSorting(tagId, sort);
// TODO: Per-account? https://github.com/vector-im/riot-web/issues/14114
localStorage.setItem(`mx_tagSort_${tagId}`, sort);
}
@ -269,11 +280,13 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
// noinspection JSMethodCanBeStatic
private getStoredTagSorting(tagId: TagID): SortAlgorithm {
// TODO: Per-account? https://github.com/vector-im/riot-web/issues/14114
return <SortAlgorithm>localStorage.getItem(`mx_tagSort_${tagId}`);
}
public async setListOrder(tagId: TagID, order: ListAlgorithm) {
await this.algorithm.setListOrdering(tagId, order);
// TODO: Per-account? https://github.com/vector-im/riot-web/issues/14114
localStorage.setItem(`mx_listOrder_${tagId}`, order);
}
@ -283,6 +296,7 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
// noinspection JSMethodCanBeStatic
private getStoredListOrder(tagId: TagID): ListAlgorithm {
// TODO: Per-account? https://github.com/vector-im/riot-web/issues/14114
return <ListAlgorithm>localStorage.getItem(`mx_listOrder_${tagId}`);
}
@ -319,6 +333,7 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
}
private onAlgorithmListUpdated = () => {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log("Underlying algorithm has triggered a list update - refiring");
this.emit(LISTS_UPDATE_EVENT, this);
};
@ -334,8 +349,10 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
}
if (this.state.tagsEnabled) {
// TODO: Find a more reliable way to get tags (this doesn't work)
// TODO: Fix custom tags: https://github.com/vector-im/riot-web/issues/14091
const roomTags = TagOrderStore.getOrderedTags() || [];
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log("rtags", roomTags);
}
@ -348,6 +365,7 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
}
public addFilter(filter: IFilterCondition): void {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log("Adding filter condition:", filter);
this.filterConditions.push(filter);
if (this.algorithm) {
@ -356,6 +374,7 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
}
public removeFilter(filter: IFilterCondition): void {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log("Removing filter condition:", filter);
const idx = this.filterConditions.indexOf(filter);
if (idx >= 0) {

2
src/stores/room-list/RoomListStoreTempProxy.ts
View file

@ -24,7 +24,7 @@ import { ITagMap } from "./algorithms/models";
* Temporary RoomListStore proxy. Should be replaced with RoomListStore2 when
* it is available to everyone.
*
* TODO: Remove this when RoomListStore gets fully replaced.
* TODO: Delete this: https://github.com/vector-im/riot-web/issues/14231
*/
export class RoomListStoreTempProxy {
public static isUsingNewStore(): boolean {

5
src/stores/room-list/TagWatcher.ts
View file

@ -23,7 +23,7 @@ import { arrayDiff, arrayHasDiff } from "../../utils/arrays";
* Watches for changes in tags/groups to manage filters on the provided RoomListStore
*/
export class TagWatcher {
// TODO: Support custom tags, somehow (deferred to later work - need support elsewhere)
// TODO: Support custom tags, somehow: https://github.com/vector-im/riot-web/issues/14091
private filters = new Map<string, CommunityFilterCondition>();
constructor(private store: RoomListStore2) {
@ -44,7 +44,7 @@ export class TagWatcher {
const newFilters = new Map<string, CommunityFilterCondition>();
// TODO: Support custom tags properly
// TODO: Support custom tags, somehow: https://github.com/vector-im/riot-web/issues/14091
const filterableTags = newTags.filter(t => t.startsWith("+"));
for (const tag of filterableTags) {
@ -61,6 +61,7 @@ export class TagWatcher {
const diff = arrayDiff(lastTags, newTags);
for (const tag of diff.added) {
// TODO: Remove this check when custom tags are supported (as we shouldn't be losing filters)
// Ref https://github.com/vector-im/riot-web/issues/14091
const filter = newFilters.get(tag);
if (!filter) continue;

23
src/stores/room-list/algorithms/Algorithm.ts
View file

@ -34,7 +34,7 @@ import { EffectiveMembership, splitRoomsByMembership } from "../membership";
import { OrderingAlgorithm } from "./list-ordering/OrderingAlgorithm";
import { getListAlgorithmInstance } from "./list-ordering";
// TODO: Add locking support to avoid concurrent writes?
// TODO: Add locking support to avoid concurrent writes? https://github.com/vector-im/riot-web/issues/14235
/**
* Fired when the Algorithm has determined a list has been updated.
@ -185,7 +185,6 @@ export class Algorithm extends EventEmitter {
// the same thing it no-ops. After we're done calling the algorithm, we'll issue
// a new update for ourselves.
const lastStickyRoom = this._stickyRoom;
console.log(`Last sticky room:`, lastStickyRoom);
this._stickyRoom = null;
this.recalculateStickyRoom();
@ -262,6 +261,8 @@ export class Algorithm extends EventEmitter {
}
}
newMap[tagId] = allowedRoomsInThisTag;
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[DEBUG] ${newMap[tagId].length}/${rooms.length} rooms filtered into ${tagId}`);
}
@ -296,6 +297,8 @@ export class Algorithm extends EventEmitter {
if (filteredRooms.length > 0) {
this.filteredRooms[tagId] = filteredRooms;
}
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[DEBUG] ${filteredRooms.length}/${rooms.length} rooms filtered into ${tagId}`);
}
@ -335,6 +338,7 @@ export class Algorithm extends EventEmitter {
}
if (!this._cachedStickyRooms || !updatedTag) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`Generating clone of cached rooms for sticky room handling`);
const stickiedTagMap: ITagMap = {};
for (const tagId of Object.keys(this.cachedRooms)) {
@ -346,6 +350,7 @@ export class Algorithm extends EventEmitter {
if (updatedTag) {
// Update the tag indicated by the caller, if possible. This is mostly to ensure
// our cache is up to date.
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`Replacing cached sticky rooms for ${updatedTag}`);
this._cachedStickyRooms[updatedTag] = this.cachedRooms[updatedTag].map(r => r); // shallow clone
}
@ -355,6 +360,7 @@ export class Algorithm extends EventEmitter {
// we might have updated from the cache is also our sticky room.
const sticky = this._stickyRoom;
if (!updatedTag || updatedTag === sticky.tag) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`Inserting sticky room ${sticky.room.roomId} at position ${sticky.position} in ${sticky.tag}`);
this._cachedStickyRooms[sticky.tag].splice(sticky.position, 0, sticky.room);
}
@ -440,10 +446,12 @@ export class Algorithm extends EventEmitter {
// Split out the easy rooms first (leave and invite)
const memberships = splitRoomsByMembership(rooms);
for (const room of memberships[EffectiveMembership.Invite]) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[DEBUG] "${room.name}" (${room.roomId}) is an Invite`);
newTags[DefaultTagID.Invite].push(room);
}
for (const room of memberships[EffectiveMembership.Leave]) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[DEBUG] "${room.name}" (${room.roomId}) is Historical`);
newTags[DefaultTagID.Archived].push(room);
}
@ -462,8 +470,10 @@ export class Algorithm extends EventEmitter {
let inTag = false;
if (tags.length > 0) {
for (const tag of tags) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[DEBUG] "${room.name}" (${room.roomId}) is tagged as ${tag}`);
if (!isNullOrUndefined(newTags[tag])) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[DEBUG] "${room.name}" (${room.roomId}) is tagged with VALID tag ${tag}`);
newTags[tag].push(room);
inTag = true;
@ -472,8 +482,10 @@ export class Algorithm extends EventEmitter {
}
if (!inTag) {
// TODO: Determine if DM and push there instead
// TODO: Determine if DM and push there instead: https://github.com/vector-im/riot-web/issues/14236
newTags[DefaultTagID.Untagged].push(room);
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[DEBUG] "${room.name}" (${room.roomId}) is Untagged`);
}
}
@ -537,8 +549,8 @@ export class Algorithm extends EventEmitter {
if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from");
if (cause === RoomUpdateCause.PossibleTagChange) {
// TODO: Be smarter and splice rather than regen the planet.
// TODO: No-op if no change.
// TODO: Be smarter and splice rather than regen the planet. https://github.com/vector-im/riot-web/issues/14035
// TODO: No-op if no change. https://github.com/vector-im/riot-web/issues/14035
await this.setKnownRooms(this.rooms);
return true;
}
@ -548,6 +560,7 @@ export class Algorithm extends EventEmitter {
// as the sticky room relies on this.
if (cause !== RoomUpdateCause.NewRoom && cause !== RoomUpdateCause.RoomRemoved) {
if (this.stickyRoom === room) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.warn(`[RoomListDebug] Received ${cause} update for sticky room ${room.roomId} - ignoring`);
return false;
}

4
src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts
View file

@ -87,6 +87,8 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
public constructor(tagId: TagID, initialSortingAlgorithm: SortAlgorithm) {
super(tagId, initialSortingAlgorithm);
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Constructed an ImportanceAlgorithm for ${tagId}`);
}
@ -292,7 +294,7 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
// "should never happen" disclaimer goes here
console.warn(`!! Room list index corruption: ${lastCat} (i:${indices[lastCat]}) is greater than ${thisCat} (i:${indices[thisCat]}) - category indices are likely desynced from reality`);
// TODO: Regenerate index when this happens
// TODO: Regenerate index when this happens: https://github.com/vector-im/riot-web/issues/14234
}
}
}

4
src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts
View file

@ -28,6 +28,8 @@ export class NaturalAlgorithm extends OrderingAlgorithm {
public constructor(tagId: TagID, initialSortingAlgorithm: SortAlgorithm) {
super(tagId, initialSortingAlgorithm);
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log(`[RoomListDebug] Constructed a NaturalAlgorithm for ${tagId}`);
}
@ -49,7 +51,7 @@ export class NaturalAlgorithm extends OrderingAlgorithm {
if (idx >= 0) this.cachedOrderedRooms.splice(idx, 1);
}
// TODO: Optimize this to avoid useless operations
// TODO: Optimize this to avoid useless operations: https://github.com/vector-im/riot-web/issues/14035
// For example, we can skip updates to alphabetic (sometimes) and manually ordered tags
this.cachedOrderedRooms = await sortRoomsWithAlgorithm(this.cachedOrderedRooms, this.tagId, this.sortingAlgorithm);

1
src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts
View file

@ -32,6 +32,7 @@ export class RecentAlgorithm implements IAlgorithm {
// of the rooms to each other.
// TODO: We could probably improve the sorting algorithm here by finding changes.
// See https://github.com/vector-im/riot-web/issues/14035
// For example, if we spent a little bit of time to determine which elements have
// actually changed (probably needs to be done higher up?) then we could do an
// insertion sort or similar on the limited set of changes.

1
src/stores/room-list/filters/CommunityFilterCondition.ts
View file

@ -52,6 +52,7 @@ export class CommunityFilterCondition extends EventEmitter implements IFilterCon
const beforeRoomIds = this.roomIds;
this.roomIds = (await GroupStore.getGroupRooms(this.community.groupId)).map(r => r.roomId);
if (arrayHasDiff(beforeRoomIds, this.roomIds)) {
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log("Updating filter for group: ", this.community.groupId);
this.emit(FILTER_CHANGED);
}

3
src/stores/room-list/filters/NameFilterCondition.ts
View file

@ -41,6 +41,7 @@ export class NameFilterCondition extends EventEmitter implements IFilterConditio
public set search(val: string) {
this._search = val;
// TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035
console.log("Updating filter for room name search:", this._search);
this.emit(FILTER_CHANGED);
}
@ -57,7 +58,7 @@ export class NameFilterCondition extends EventEmitter implements IFilterConditio
}
}
if (!room.name) return false; // should realisitically not happen: the js-sdk always calculates a name
if (!room.name) return false; // should realistically not happen: the js-sdk always calculates a name
// Note: we have to match the filter with the removeHiddenChars() room name because the
// function strips spaces and other characters (M becomes RN for example, in lowercase).

2
src/stores/room-list/membership.ts
View file

@ -63,7 +63,7 @@ export function getEffectiveMembership(membership: string): EffectiveMembership
if (membership === 'invite') {
return EffectiveMembership.Invite;
} else if (membership === 'join') {
// TODO: Do the same for knock? Update docs as needed in the enum.
// TODO: Include knocks? Update docs as needed in the enum. https://github.com/vector-im/riot-web/issues/14237
return EffectiveMembership.Join;
} else {
// Probably a leave, kick, or ban