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

Make Converse look more familiar to some users. #1167

Merged
merged 1 commit into from Nov 14, 2018

Conversation

Projects
None yet
5 participants
@linkmauve
Contributor

linkmauve commented Aug 17, 2018

This is totally not inspired by https://discordapp.com/ :-° Hopefully
copyright doesn’t apply to a few CSS values, but this should be checked
before merging this PR.

Also I didn’t take much advantage of SCSS variables, so there is a lot
of duplication for colour values and such, this should be fixed before
this can be merged.

There is also no theme support in Converse yet, we may want to fix that
for people who prefer the old look.

Here is how it currently looks:
converse

@linkmauve linkmauve force-pushed the linkmauve:discord-look branch from db7c117 to a76c502 Aug 21, 2018

@Nyco

This comment has been minimized.

Member

Nyco commented Aug 28, 2018

Wow, thanks a lot for this @linkmauve ! ;-)

Regarding IP/copyrights, if there is a real risk, can we change what we estimate might hurt?

I have the feeling (hopefully neutral) that it looks better! ;-) My opinion is: it definitely looks better!

Another opinion: we have to be careful with "too grey-ish" colors, we might want to present a more colourful look.

Another personal observation, it seems to me that the world separates in two, light vs dark modes... And be careful, lovers/hater seem very opinioniated ;-)

Can you please post a screenshot in mobile mode?
Update: The vertical spacing between the conversations list items is not large enough for finger-pointing devices such as mobile smartphones and tablets (some laptops are multi-touch as well).

@jcbrand

This comment has been minimized.

Member

jcbrand commented Aug 28, 2018

Yes, thanks @linkmauve

Quite a number of people have spoken positively about this change.

There is also no theme support in Converse yet, we may want to fix that for people who prefer the old look.

Inasmuch as you can edit the variables in _converse.scss and generate a new CSS file, there is theme support. The challenge is to make this simpler, more user-friendly and exposing this to end-users.

One way would be to use CSS variables instead of Sass. That would mean dropping support for IE11.

The user could then choose a theme themselves, and have Converse automatically update.
With Sass this is not possible because there's a compilation step necessary.

Also I didn’t take much advantage of SCSS variables, so there is a lot of duplication for colour values and such, this should be fixed before this can be merged.

I consider this a blocker to merging this PR. The current Sass variables might not be enough, or used properly, but the solution is to fix them, not to ignore them.

Ideally this pull request would just be another variables file, which specifies a different theme.
That might not be possible in this case (i.e. other files might also have to be modified), but that's the goal we should be working towards IMO.

Concerning what @Nyco wrote:

Another opinion: we have to be careful with "too grey-ish" colors, we might want to present a more colourful look.

Depends on the audience. I too prefer more colour, but some people like things darker.
That's another reason why better theme support would be helpful, so that people can choose for themselves how it should look.


Lastly, I have asked a designer to come up with a new logo for Converse and in the process a new colour scheme.

I expect that a new theme might come from his work, and that this would become the default theme.

However, with support for multiple themes, we can still accommodate this PR and any other themes people wish to submit.

@Nyco

This comment has been minimized.

Member

Nyco commented Aug 28, 2018

That would mean dropping support for IE11.

Good riddance, it is at 2.64% market share a the time of this writing.
https://caniuse.com/usage-table

@linkmauve

This comment has been minimized.

Contributor

linkmauve commented Aug 28, 2018

This work was mostly a proof of concept (hence the WIP flag) because it was the most common criticism I heard about Converse after deploying it here and there, it is definitely not intended to be merged as is, but I’m already using it and my users generally prefer it to the current one.

Ignoring SCSS variables was only a way for me to get this done easily, if I do put more work into this the first thing to do will be to extract every place where I used the same colour and make that a variable. I could also try to copy sass/_variables.scss into another file and call it a theme.

As for the theme itself, I don’t especially like it either, I just chose Discord as a model because I’ve heard a lot of gamers use it, one of the communities I helped migrate to XMPP was about game development, and I thought some familiarity would help. Note Nÿco that I didn’t go to the very end of having a clone, the original has support for dark/light themes, a separator between each message and not just different days, etc.

I don’t own a mobile device, so at best I could screenshot Firefox’s devtools in responsive mode, instead you can try JabberFR’s deployment at https://chat.jabberfr.org/ (click on « Rejoindre », “join” in French, for a room you want to test).

I probably won’t work on reworking both themes to use the same SCSS code and only change variables in the few next weeks, but if there is interest in getting this merged before your designer comes up with something I can definitely priorise it more.

@conversejs conversejs deleted a comment from Mukeshstl Sep 13, 2018

@jcbrand jcbrand force-pushed the conversejs:master branch 2 times, most recently from 81498d6 to 6bab16d Oct 29, 2018

@linkmauve linkmauve force-pushed the linkmauve:discord-look branch 2 times, most recently from 23441c7 to f31cfab Oct 31, 2018

@linkmauve

This comment has been minimized.

Contributor

linkmauve commented Oct 31, 2018

This has been rebased on top of #1268, using custom properties instead of hardcoding any value. The two themes can now be toggled just by adding or removing the theme-discord class on #conversejs. I’m not too sure if this selector is correct because it made me add an !important on every CSS rule, but as a WIP this should work much better than the existing version.

I also have a sizing issue between the chat-area and the occupants divs, which I’ve been unable to fix.

@linkmauve linkmauve force-pushed the linkmauve:discord-look branch 5 times, most recently from bdbdfe9 to ddcb4e1 Oct 31, 2018

@linkmauve linkmauve changed the title from WIP: Make Converse look more familiar to some users. to Make Converse look more familiar to some users. Nov 1, 2018

@linkmauve linkmauve force-pushed the linkmauve:discord-look branch 3 times, most recently from ec488c4 to 044ec57 Nov 2, 2018

@linkmauve linkmauve force-pushed the linkmauve:discord-look branch 2 times, most recently from 94f5214 to 25e5156 Nov 13, 2018

@jcbrand

I don't see where the file sass/discord.css is pulled into the CSS build. That should be in sass/converse.scss.

Looks like you're including this file manually in your HTML, is that so?

Show resolved Hide resolved sass/discord.css Outdated
Add a dark theme inspired by Discord
This theme takes inspiration from https://discordapp.com/ and builds on
top of custom-properties, making the design more familiar to some users.

In order to change the theme, add the 'theme-dark' class on #conversejs,
you can do it at any point during the lifetime of Converse, either
directly in your HTML or by changing the DOM at runtime.

@linkmauve linkmauve force-pushed the linkmauve:discord-look branch from 25e5156 to a5c7727 Nov 14, 2018

@jcbrand jcbrand merged commit 174b257 into conversejs:master Nov 14, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jcbrand

This comment has been minimized.

Member

jcbrand commented Nov 14, 2018

Thanks!

@stevenroose

This comment has been minimized.

stevenroose commented Nov 23, 2018

Could this be followed up with a README/wiki update on how to use it? :)

Very good work! IMO something like this could become default.

@licaon-kter

This comment has been minimized.

Contributor

licaon-kter commented Nov 23, 2018

@licaon-kter

This comment has been minimized.

Contributor

licaon-kter commented Nov 23, 2018

@linkmauve Only MUCs were styled?

1:1 still (ugly) green
cxontact

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment