Ensure list visibility changes get counted as list changes

Fixes https://github.com/vector-im/riot-web/issues/14799

We were checking to see if the tags were visible at render time, but we needed to ensure that they were(n't) included when checking for diffs. This introduces a new kind of object cloning for semantic reasons.

This also fixes the selection indicator being a bit off on custom tags.
This commit is contained in:
Travis Ralston 2020-07-30 14:18:54 -06:00
parent e953bfbf88
commit 5b15d12865
3 changed files with 28 additions and 6 deletions

View file

@ -54,5 +54,5 @@ limitations under the License.
position: absolute;
left: -9px;
border-radius: 0 3px 3px 0;
top: 12px; // just feels right (see comment above about designs needing to be updated)
top: 5px; // just feels right (see comment above about designs needing to be updated)
}

View file

@ -42,7 +42,7 @@ import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNo
import SettingsStore from "../../../settings/SettingsStore";
import CustomRoomTagStore from "../../../stores/CustomRoomTagStore";
import { arrayFastClone, arrayHasDiff } from "../../../utils/arrays";
import { objectShallowClone } from "../../../utils/objects";
import { objectShallowClone, objectWithOnly } from "../../../utils/objects";
interface IProps {
onKeyDown: (ev: React.KeyboardEvent) => void;
@ -220,7 +220,12 @@ export default class RoomList extends React.PureComponent<IProps, IState> {
}
const previousListIds = Object.keys(this.state.sublists);
const newListIds = Object.keys(newLists);
const newListIds = Object.keys(newLists).filter(t => {
if (!isCustomTag(t)) return true; // always include non-custom tags
// if the tag is custom though, only include it if it is enabled
return CustomRoomTagStore.getTags()[t];
});
let doUpdate = arrayHasDiff(previousListIds, newListIds);
if (!doUpdate) {
@ -240,7 +245,8 @@ export default class RoomList extends React.PureComponent<IProps, IState> {
if (doUpdate) {
// We have to break our reference to the room list store if we want to be able to
// diff the object for changes, so do that.
const sublists = objectShallowClone(newLists, (k, v) => arrayFastClone(v));
const newSublists = objectWithOnly(newLists, newListIds);
const sublists = objectShallowClone(newSublists, (k, v) => arrayFastClone(v));
this.setState({sublists}, () => {
this.props.onResize();
@ -288,8 +294,7 @@ export default class RoomList extends React.PureComponent<IProps, IState> {
const tagOrder = TAG_ORDER.reduce((p, c) => {
if (c === CUSTOM_TAGS_BEFORE_TAG) {
const customTags = Object.keys(this.state.sublists)
.filter(t => isCustomTag(t))
.filter(t => CustomRoomTagStore.getTags()[t]); // isSelected
.filter(t => isCustomTag(t));
p.push(...customTags);
}
p.push(c);

View file

@ -36,6 +36,23 @@ export function objectExcluding(a: any, props: string[]): any {
}, {});
}
/**
* Gets a new object which represents the provided object, with only some properties
* included.
* @param a The object to clone properties of. Must be defined.
* @param props The property names to keep.
* @returns The new object with only the provided properties.
*/
export function objectWithOnly(a: any, props: string[]): any {
const existingProps = Object.keys(a);
const diff = arrayDiff(existingProps, props);
if (diff.removed.length === 0) {
return objectShallowClone(a);
} else {
return objectExcluding(a, diff.removed);
}
}
/**
* Clones an object to a caller-controlled depth. When a propertyCloner is supplied, the
* object's properties will be passed through it with the return value used as the new