Center loading screen and add logo #1668

xPaw · 2017-10-28T20:52:51Z

Before	After

Javascript message was broken in 1d47290

MaxLeiter · 2018-01-31T06:03:54Z

client/index.html

@@ -95,7 +88,7 @@ <h1 class="title" id="loading-title">The Lounge is loading…</h1>
 	<div id="image-viewer"></div>

 	<script src="js/bundle.vendor.js"></script>
-	<script src="js/bundle.js"></script>
+	<script src="js/bundlez.js"></script>


accidental z?

AlMcKinlay

Yeah, I like it a bit better.

astorije · 2018-02-28T06:24:57Z

client/css/style.css

 }

 #loading p {
 	margin-top: 10px;
 }

+#loading img {
+	max-width: 256px;


Would you mind adding a max-height or something (Chrome tells me the image shows as 256px x 171.48px)? On load there is a bump until the image is loaded, which is right in the middle of loading (at least in my case).
I'm playing with slow network and it does that too.

astorije · 2018-02-28T06:26:01Z

This is so nice!

I noticed that on screens that aren't very high, like landscape mode on mobile, it tends to do this:

How expensive would it be to fix this?

xPaw · 2018-02-28T17:37:46Z

@astorije Fixed it up by further using flexbox instead of absolute positioning.

I also set exact size on the image so the jumping doesn't happen. Can you confirm it's fine now?

astorije · 2018-03-01T04:49:13Z

client/index.html.tpl

-							</div>
-						</div>
+					<div id="loading-status-container">
+						<img src="img/logo-vertical-transparent-bg.svg" alt="The Lounge" width="256" height="170">


I often forget, but we allow server config to specify a custom theme, which is then used on loading screen. When theme is dark, result is pretty horrible:

What about having a second image here (this one) hidden with CSS, then custom themes can hide the normal one and display inverted logo only?

(Alternatively, get rid of server config for theme and only have official theme on page load... I know, I know, this idea is not liked 😅)

I know, I know, this idea is not liked

It's not liked because those of us using dark themes don't like a bright flash when loading the lounge. If we were to get custom themes to sync from client to server, then I'd be fine with removing the server theme. Only then.

Also, bear in mind that even if you did remove the server theme, the client theme takes over before this screen goes away sometimes, so this still needs to be dealt with.

That was not a real suggestion, just a bit of trolling :D
Real suggestion is the 2 images, but there are probably others (css background?)

I know you were trolling, I'm just making sure people know the reasons for it :-P

As for the actual feature...I'm not sure which I prefer. The hidden image is easiest for theme makers, but also most feels like a hack. Maybe just making both images available on the server and themes can set css background, as you say. Shouldn't be too difficult?

Other option (this is bigger than this PR I think) is we can have a way for themes to set themselves as "light" or "dark" and they can inherit things like this from defaults.

We could do some color changing dynamically (e.g. from manifest or background color) as it's svg.

That could also work.

astorije · 2018-03-01T04:50:14Z

Very nice fixes @xPaw, good job!

xPaw · 2018-03-01T11:53:01Z

Okay I've embedded the SVG in html itself, this allows themes to individually change colors of the logo.

dgw · 2018-03-01T13:15:22Z

How does letting themes change the logo colors square with the brand guidelines given by the logo designer in #282? Specifically, "Don't alter or change the colors".

astorije · 2018-03-02T03:53:17Z

What @dgw said is on point. Let's not do this, it would be a recipe to disaster.
If someone's theme does not work with a given background, then they should hide the logo instead of altering it.

Let's go with either having 2 images, or choosing the image via CSS background like @McInkay mentioned. I don't have a preference over which of those solutions though.

xPaw · 2018-03-02T07:41:19Z

Brand guideline or not, I think this is a slightly more elegant solution, as opposed to having to switch images (either there are 2 img tags in the page, or change background-image).

astorije · 2018-03-02T07:44:43Z

That requires theme authors to set up our specific colors as opposed to pick an image, I don't find that elegant at all from a theme author perspective. Also not very elegant to embed the SVG code within the markup IMO.
And I am 💯% sure that themes will go nuts on the logo colors.

I think we really shouldn't do that.

xPaw · 2018-03-02T21:01:16Z

@McInkay which method do you prefer? Coloring individual parts via CSS (svg is embedded in html) or changing background image via css?

I don't want to put 2 tags because then the browser will start downloading both images.

AlMcKinlay · 2018-03-02T21:03:31Z

Euch.

I think I'm probably with @astorije - we have strict guidelines around the logo, so we shouldn't encourage breaking the guidelines. So probably background css.

xPaw · 2018-03-02T21:04:29Z

Mind you, changing image url won't stop theme designers from switching to any other logo they've designed (so they could copy the svg and change the colors there)...

astorije · 2018-03-02T21:11:55Z

All things considered, I would prefer having both <img> elements over background CSS, and way way over SVG + custom colors. Optimized SVG file is 2.62 KB, 475 bytes transferred over the network (according to Chrome devtools). I think we can live with the overhead :)
(EDIT: Looks like 1,175 bytes actually, but either way :bikeshed:)

xPaw force-pushed the xpaw/center-loading branch from 8f3d099 to bb57165 Compare November 20, 2017 10:31

xPaw requested a review from astorije December 3, 2017 15:08

xPaw force-pushed the xpaw/center-loading branch from bb57165 to 53158e9 Compare December 11, 2017 17:58

xPaw added the Type: Feature Tickets that describe a desired feature or PRs that add them to the project. label Dec 21, 2017

astorije added this to the 3.0.0 milestone Jan 19, 2018

MaxLeiter reviewed Jan 31, 2018

View reviewed changes

xPaw force-pushed the xpaw/center-loading branch from 53158e9 to ad35230 Compare January 31, 2018 08:21

xPaw assigned astorije Feb 19, 2018

xPaw force-pushed the xpaw/center-loading branch from ad35230 to 43eec14 Compare February 20, 2018 09:13

AlMcKinlay approved these changes Feb 27, 2018

View reviewed changes

astorije reviewed Feb 28, 2018

View reviewed changes

xPaw force-pushed the xpaw/center-loading branch 2 times, most recently from 91ea13f to 85ac9c2 Compare February 28, 2018 17:37

xPaw force-pushed the xpaw/center-loading branch from 85ac9c2 to f16b612 Compare February 28, 2018 17:43

astorije reviewed Mar 1, 2018

View reviewed changes

xPaw force-pushed the xpaw/center-loading branch from ea1a330 to 15e07b8 Compare March 1, 2018 11:54

xPaw force-pushed the xpaw/center-loading branch from 15e07b8 to cb1bfc1 Compare March 2, 2018 21:38

Center loading screen

276266e

xPaw force-pushed the xpaw/center-loading branch from cb1bfc1 to 276266e Compare March 3, 2018 07:45

astorije approved these changes Mar 3, 2018

View reviewed changes

astorije changed the title ~~Center loading screen~~ Center loading screen and add logo Mar 3, 2018

xPaw merged commit e0b15f1 into master Mar 3, 2018

xPaw deleted the xpaw/center-loading branch March 3, 2018 08:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Center loading screen and add logo #1668

Center loading screen and add logo #1668

xPaw commented Oct 28, 2017 •

edited

Loading

MaxLeiter Jan 31, 2018

AlMcKinlay left a comment

astorije Feb 28, 2018

astorije commented Feb 28, 2018

xPaw commented Feb 28, 2018 •

edited

Loading

astorije Mar 1, 2018

AlMcKinlay Mar 1, 2018

astorije Mar 1, 2018

AlMcKinlay Mar 1, 2018

xPaw Mar 1, 2018

AlMcKinlay Mar 1, 2018

astorije commented Mar 1, 2018

xPaw commented Mar 1, 2018

dgw commented Mar 1, 2018

astorije commented Mar 2, 2018

xPaw commented Mar 2, 2018

astorije commented Mar 2, 2018

xPaw commented Mar 2, 2018

AlMcKinlay commented Mar 2, 2018

xPaw commented Mar 2, 2018

astorije commented Mar 2, 2018 •

edited

Loading

Center loading screen and add logo #1668

Center loading screen and add logo #1668

Conversation

xPaw commented Oct 28, 2017 • edited Loading

Choose a reason for hiding this comment

AlMcKinlay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astorije commented Feb 28, 2018

xPaw commented Feb 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astorije commented Mar 1, 2018

xPaw commented Mar 1, 2018

dgw commented Mar 1, 2018

astorije commented Mar 2, 2018

xPaw commented Mar 2, 2018

astorije commented Mar 2, 2018

xPaw commented Mar 2, 2018

AlMcKinlay commented Mar 2, 2018

xPaw commented Mar 2, 2018

astorije commented Mar 2, 2018 • edited Loading

xPaw commented Oct 28, 2017 •

edited

Loading

xPaw commented Feb 28, 2018 •

edited

Loading

astorije commented Mar 2, 2018 •

edited

Loading