Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

hide empty roomsublists when filtering via search/tagpanel #1954

Merged
merged 2 commits into from
Jun 25, 2018

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Jun 12, 2018

@lukebarnard1 lukebarnard1 self-assigned this Jun 13, 2018
@@ -347,8 +347,14 @@ var RoomSubList = React.createClass({
var label = this.props.collapsed ? null : this.props.label;

let content;
if (this.state.sortedList.length === 0 && !this.props.searchFilter && this.props.extraTiles.length === 0) {
content = this.props.emptyContent;
if (this.state.sortedList.length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not include && this.props.extraTiles.length === 0 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup you're right, mine worked by an assumption that the current uses of <RoomSubList /> are the only possible use cases but in future, we may have alternate ones.

if (this.state.sortedList.length === 0 && !this.props.searchFilter && this.props.extraTiles.length === 0) {
content = this.props.emptyContent;
if (this.state.sortedList.length === 0) {
// check if emptyContent is defined, if falsey then don't show an empty sublist even without searchFilter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, this comment makes things more confusing. I would at least write it in the order of conditions, i.e. "If no search filter is applied and ..." or "Only show placeholder if ..."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done thanks

@lukebarnard1 lukebarnard1 assigned t3chguy and unassigned lukebarnard1 Jun 18, 2018
@t3chguy t3chguy assigned lukebarnard1 and unassigned t3chguy Jun 19, 2018
// case insensitive if room name includes filter,
// or if starts with `#` and one of room's aliases starts with filter
return list.filter((room) => (room.name && room.name.toLowerCase().includes(lcFilter)) ||
(filter[0] === '#' && room.getAliases().some((alias) => alias.toLowerCase().startsWith(lcFilter))));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look like a merge commit, it includes changes to include matches against room aliases.

That's fine, but let's squash this PR (when merging) for simplicity.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@lukebarnard1 lukebarnard1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it would be clearer to separate the delint from the actual changes. @t3chguy, is there any chance you could squash the actual change commits into a single commit? That should leave us with a single delint commit and a single commit for actual changes.

Sorry for faff.

@lukebarnard1 lukebarnard1 assigned t3chguy and unassigned lukebarnard1 Jun 21, 2018
@t3chguy t3chguy force-pushed the t3chguy/hide_empty_sublist branch from 2fd5833 to fabdf22 Compare June 25, 2018 08:54
@t3chguy
Copy link
Member Author

t3chguy commented Jun 25, 2018

restaged things for you @lukebarnard1

@t3chguy t3chguy assigned lukebarnard1 and unassigned t3chguy Jun 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants