Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix two room list regressions #4907

Merged
merged 2 commits into from
Aug 29, 2017

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Aug 27, 2017

  • missing roomsublist badge for invites
  • missing room badge for invites if i18n!=English

Requires matrix-org/matrix-react-sdk#1343

Signed-off-by: Michael Telatynski [email protected]

+ missing roomsublist badge for invites
+ missing room badge for invites if i18n!=English

Signed-off-by: Michael Telatynski <[email protected]>
@@ -78,6 +78,9 @@ var RoomSubList = React.createClass({
// undefined if no room is selected (eg we are showing settings)
selectedRoom: React.PropTypes.string,

// pass explicitly, do not rely on label==='Invites' because i18n.
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of context, this comment doesn't mean much. Maybe just mention that it's passed through to RoomTile and makes the header highlighted regardless of notifications.

@@ -409,6 +412,9 @@ var RoomSubList = React.createClass({
var badge;
if (subListNotifCount > 0) {
badge = <div className={badgeClasses}>{ FormattingUtils.formatCount(subListNotifCount) }</div>;
} else if (this.props.isInvite) {
// no notifications but highlight anyway because this is an invite badge
badge = <div className={badgeClasses}>!</div>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps show the number of invites, i.e. the number of items in the sublist. I'd suggest just using the if, passing a generic count to .formatCount above, and then deciding whether to set count to subListNotifCount or roomCount based on isInvite.

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind, the "!" is by design and the number of invites is already displayed in the sublist header.

Copy link
Member Author

Choose a reason for hiding this comment

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

#3707 (comment) for history sake

Signed-off-by: Michael Telatynski <[email protected]>
@t3chguy t3chguy force-pushed the t3chguy/fix_two_roomlist_regressions branch from 49dee4d to f560dc4 Compare August 29, 2017 13:59
@t3chguy
Copy link
Member Author

t3chguy commented Aug 29, 2017

@lukebarnard1 lukebarnard1 merged commit 2ad24a0 into develop Aug 29, 2017
@t3chguy t3chguy deleted the t3chguy/fix_two_roomlist_regressions branch October 29, 2017 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants