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

Implement default room colors #1479

Closed
wants to merge 4 commits into from

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Oct 14, 2017

Signed-off-by: Travis Ralston <travpc@gmail.com>
Signed-off-by: Travis Ralston <travpc@gmail.com>
Using /tint without a second color means the secondary_color is null/falsey. Previously, the code didn't treat it as optional.

Signed-off-by: Travis Ralston <travpc@gmail.com>
@ara4n
Copy link
Member

ara4n commented Oct 14, 2017

this looks awesome! but shouldn't the default room colour picker be in UserSettings rather than RoomSettings, given it impacts the whole app rather than just that specific room?

@turt2live
Copy link
Member Author

turt2live commented Oct 14, 2017

The default room color is per-room, to implement element-hq/element-web#738 ("let room admins set default room color"). It's backed by a state event in the room, and the user's room account data will override the state event's choices.

Although this PR could be extended to add a 3rd color picker to UserSettings to have blanket defaults for all rooms. Is that something that would be wanted?

@ara4n
Copy link
Member

ara4n commented Oct 14, 2017

aaaaah, i see. hmm. i wonder how much we really want room admins setting their own weird white-on-white style colour schemes for rooms... (and yes, i know i asked for it in the first place).

I also wonder if this should be implemented using generic granular settings (i.e. the ability to specify settings arbitrarily per-user, per-room, globally - also useful for stuff like configuring hiding URL previews, join/parts and other cosmetics)

@turt2live
Copy link
Member Author

It should be fine if room admins want to do that, as users can always override their choice. The room admin may also find themselves with an angry crowd of people or an empty room - which might be enough of a deterrent to going outside the color schemes Riot lets you define.

Granular settings sounds like something @t3chguy has promised for quite a while now :p
It really should be a granular setting, but I don't have all the details to implement it myself. If the requirements are outlined somewhere (and maybe some implementation notes?) I could start working away on it if @t3chguy is okay with it.

@t3chguy
Copy link
Member

t3chguy commented Oct 14, 2017

Granular Settings are still on the horizon (distant distant horizon)
I need someone to give me an inspiration of how the UI/UX for them would look without drowning everyone with too much choice

@t3chguy
Copy link
Member

t3chguy commented Oct 14, 2017

as for notes on Granular Settings I will try to recite part of a conversation between Matthew, Toml and I,


Per-room per-device
Per-room account
Per-room room state default


Per device
Per account
Default

is the ordering of the possible sources of truth in Granular Settings as far as I am aware
the top half only applies to options which make sense to be room specific, such as url previews

as for whether I am okay with it, of course

@turt2live turt2live mentioned this pull request Oct 23, 2017
15 tasks
@turt2live
Copy link
Member Author

I'm blocking this on #1516 (Granular Settings).

@turt2live
Copy link
Member Author

Closing this for now - it requires relatively drastic changes to work with granular settings and I'm completely out of spare time.

@turt2live turt2live closed this Mar 14, 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.

3 participants