BZ#1471301 - Login/nav brand and logo are set by api

[AllenBW]

fixes https://bugzilla.redhat.com/show_bug.cgi?id=1471301

In dev, these things will appear as broken images, but upon build, they show up just fine

Styling on login is a little jacked up atm... but we're applying the same classes so 🤷‍♂️

What we look like (note the url ui/service)

[skateman]

@miq-bot add_reviewer @epwinchell

[himdel]

Soo.. the problem with this is that "skins" work by replacing a few things, including:

• Text.login.brand ( = vm.text.brand )
• images/login-screen-logo.png
• images/brand.svg

It looks like the API is serving the right images in that case, so the images should be fine.
The brand is missing the "Self Service" at the end now.

We should also update skin-sample/README.md and remove brand.svg and login-screen-logo.png from skin-sample/images/.
Oh, and from client/assets/images/.

But.. I'm still seeing

client/app/core/navigation/navigation-controller.js
131:      imgSrc: 'images/login-screen-logo.png',

so maybe that one should also be using brandingInfo now?

[AllenBW]

Great points @himdel !! Updated pr as per feedback... what do we think about the Text skinning in general? Should that be replaced by name or name_full provided by the api in product_info?

@skateman i think the only thing thats kinda ah bummer, we're not setting brands and logos specific to the sui... so whatever is set for cui is same here... so yeah, we're missing self service text from logo :-/

[skateman]

@himdel can we actually have skins since we're not exposing JS precompilation?

[miq-bot]

Checked commit AllenBW@674b420 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🍰

[himdel]

Thanks! :) Still seeing

client/assets/images/brand.svg
client/assets/images/login-screen-logo.png

that are not used now.

As for text, I kinda like the idea of SUI not caring about skins at all (except for pure CSS) and just getting all that from the API, so maybe we can just tack on +Self Service? (Although, not sure about the i18n aspect of that..)

[himdel]

@skateman Yes we can, they (whoever) just have to build it themselves :)

[AllenBW]

Leavin' in the images wasn't an oversight, it was intentional, to reduce pr commotion and then who knows, maybe someone will want em? But if yah really really want me to remove em, will do @himdel

[AllenBW]

@himdel @epwinchell this g2g? @Loicavenel is.... https://bugzilla.redhat.com/show_bug.cgi?id=1471301#c22

[himdel]

Aah, thanks :)

Just one last think, I think... it's never using the images now...

/api/product_info returns them under branding_info.brand,
but the code is only looking at product_info.brand...

I'm guessing we need...

diff --git a/client/app/core/appliance-info.service.js b/client/app/core/appliance-info.service.js
index b195e911..98f741d1 100644
--- a/client/app/core/appliance-info.service.js
+++ b/client/app/core/appliance-info.service.js
@@ -27,8 +27,8 @@ export function ApplianceInfo ($sessionStorage) {
       server: data.server_info.appliance,
       asyncNotify: data.settings.asynchronous_notifications || true,
       nameFull: data.product_info.name_full,
-      brand: data.product_info.brand,
-      logo: data.product_info.logo
+      brand: data.product_info.branding_info.brand,
+      logo: data.product_info.branding_info.logo
     }
 
     $sessionStorage.applianceInfo = applianceInfo

[himdel]

Verified I'm now seeing requests for those images in the console 👍

[himdel]

Looks like supporting showing the images in devel mode is non-trivial, we could make webpack-dev-server proxy /assets to the rails server and that should work fine ... except SUI is also using assets so the rest of the images would stop working instead :).

(Or maybe we can move all the images to CUI and just do that, but not sure that's the right direction, unless there's a plan to merge the 2.)

[AllenBW]

sad emoji, yeah gonna kinda given up on em workin in dev, maybe it'l be worth it to make a note in dev docs, for posterity n stuff

[simaishi]

@AllenBW Can this be hammer/yes?

[AllenBW]

@simaishi should be a clean merge as there hasn't been too much churn on the files this touches.. 🤔

[simaishi]

Hammer backport details:

$ git log -1
commit dbb82f6020e0dfd7b66ef139f0b4ce83f4e6b96c
Author: Martin Hradil <himdel@seznam.cz>
Date:   Fri Jan 4 14:09:11 2019 +0100

    Merge pull request #1513 from AllenBW/BZ/#1471301_ref-hide-top-left-brand
    
    BZ#1471301 - Login/nav brand and logo are set by api
    
    (cherry picked from commit c385bad4fdfcd2f397dfded662201ba63d7655c6)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1693757