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

Bring back custom tags, also badges on communities #2575

Merged
merged 18 commits into from
Feb 7, 2019

Conversation

bwindels
Copy link
Contributor

@bwindels bwindels commented Feb 5, 2019

element-hq/element-web#8060

Adds a toggle button in the LLP for every custom tag, which makes them appear in the the RoomList.

Questions?

  • should we also be able to disable favourites, low prio, ... like this?
  • how does the avatar prefix trimming algorithm perform in real scenarios?

Also, it has a max-height of 40vh so it won't take up more than 40% of the viewport. I couldn't make scrollbars only appear when the combination of the 3 components doesn't fit on the screen because gemini scrollbars uses position: absolute and thus can't be sized by it's content. Didn't want to port it for now...

customtags2
llpnotifbadges

@ara4n
Copy link
Member

ara4n commented Feb 5, 2019

does it roll up the badges from the rooms?

i don’t think we should put low priority there otherwise everyone will be wanting to add custom tags

@bwindels bwindels requested a review from a team February 5, 2019 19:19
@bwindels
Copy link
Contributor Author

bwindels commented Feb 5, 2019

does it roll up the badges from the rooms?

You mean if it aggregates the badges from the rooms it contains? No, but that would probably be a good idea ...

@turt2live
Copy link
Member

/me resists the urge to hit approve without checking the diff

@ara4n
Copy link
Member

ara4n commented Feb 5, 2019

does it roll up the badges from the rooms?

You mean if it aggregates the badges from the rooms it contains? No, but that would probably be a good idea ...

I think this is a prerequisite i'm afraid, otherwise people are just going to miss msgs from the rooms. (We'll need it for communities too)

@dbkr
Copy link
Member

dbkr commented Feb 6, 2019

I guess we wait for the badge stuff before reviewing this (and also whatever has angered the unit test gods today).

this._state = Object.assign({}, {tags: this._getUpdatedTags()});

this._roomListStoreToken = RoomListStore.addListener(() => {
// UGLY: FluxStore doens't emit changes that
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if there is a better way to create a store that derives it updates from another store like this?

Copy link
Member

Choose a reason for hiding this comment

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

When I wanted to do this before I made a thing called an 'Observer' which is entirely a thing I made up, but served as a thing that doesn't store data itself but aggregates updates from other stores (ActiveRoomObserver.js).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went for an "ad-hoc" store in the end, based off GroupStore, as a store seems to be closer to what I'm doing than an observer. 👍👎?

Copy link
Member

Choose a reason for hiding this comment

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

yep, wfm 👍

@bwindels bwindels changed the title Bring back custom tags Bring back custom tags + badges on communities Feb 6, 2019
@bwindels bwindels changed the title Bring back custom tags + badges on communities Bring back custom tags, also badges on communities Feb 6, 2019
@bwindels
Copy link
Contributor Author

bwindels commented Feb 6, 2019

ready for another look

Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

generally lgtm

res/css/structures/_TagPanelButtons.scss Outdated Show resolved Hide resolved
src/components/structures/CustomRoomTagPanel.js Outdated Show resolved Hide resolved
src/components/structures/CustomRoomTagPanel.js Outdated Show resolved Hide resolved
src/components/structures/CustomRoomTagPanel.js Outdated Show resolved Hide resolved
src/components/structures/CustomRoomTagPanel.js Outdated Show resolved Hide resolved
src/components/structures/CustomRoomTagPanel.js Outdated Show resolved Hide resolved
src/components/structures/CustomRoomTagPanel.js Outdated Show resolved Hide resolved
src/components/structures/TagPanelButtons.js Outdated Show resolved Hide resolved
src/stores/CustomRoomTagStore.js Outdated Show resolved Hide resolved
this._state = Object.assign({}, {tags: this._getUpdatedTags()});

this._roomListStoreToken = RoomListStore.addListener(() => {
// UGLY: FluxStore doens't emit changes that
Copy link
Member

Choose a reason for hiding this comment

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

When I wanted to do this before I made a thing called an 'Observer' which is entirely a thing I made up, but served as a thing that doesn't store data itself but aggregates updates from other stores (ActiveRoomObserver.js).

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.

4 participants