[RFE] Allow to upload and display a custom brand image on login/header #4376

skateman · 2018-07-30T11:26:45Z

This PR creates a brand image upload feature under the custom logo tab, eliminating the need to manually change the html, css, and drop images into the public folder. The image appears on the login screen and the main header. The code in the area was awful, so I also did a little refactoring.

@miq-bot add_reviewer @epwinchell
@miq-bot add_reviewer @martinpovolny
@miq-bot add_label pending core

Depends on: ManageIQ/manageiq#17773
RFE BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1471301

skateman · 2018-07-30T11:27:08Z

app/views/ops/_settings_custom_logos_tab.html.haml

+        = render :partial => "shared/file_chooser", :locals => {:object_name => "brand", :method => "logo"}
+      .col-md-6
+        = submit_tag(_("Upload"), :class => "btn btn-default")
+        = _("* Requirements: File-type - PNG; Dimensions - AAAAxBBBB.")


@epwinchell I need the sizes from you here 😉 🍻

epwinchell · 2018-07-30T13:50:00Z

@miq-bot add_reviewer @dclarizio

h-kataria · 2018-08-01T15:22:31Z

@skateman can you provide before/after screenshots

epwinchell · 2018-08-01T15:51:55Z

cc @bascar @Loicavenel

Loicavenel · 2018-08-01T16:28:12Z

Seems ok for me, can we get UX review ?

epwinchell · 2018-08-01T16:57:03Z

@Loicavenel The upload UX hasn't changed. There's nothing new here from that perspective.

Loicavenel · 2018-08-01T17:21:48Z

@epwinchell logo was on the right before... and on the left

epwinchell · 2018-08-01T17:27:07Z

@Loicavenel This just creates an upload feature for the existing brand image on the top left. The custom logo on the top right hasn't changed.

Loicavenel · 2018-08-02T12:33:05Z

@epwinchell here my issue here:

1 - Now, we can have 2 logos one of the left, one of the right...

2 - We are removing Red Hat logo, I don't know the rules in RH for this.. it is why I want to UX involved.. I don't know if other products are doing this.

@Rohoover can you help me here...

dclarizio · 2018-08-02T13:52:11Z

@terezanovotna (CF UX), can you take a look at this as well? . . . I'm pretty sure we allow customers to totally use their own branding if they choose.
@Loicavenel @bascar don't we have VM hosting providers that use us under the covers with their own branding?

bascar · 2018-08-02T13:58:58Z

We do have customers who do their own branding. They have for years and in the past, it was easier. Some recent changes have made that more difficult for re-branding and this change will allow them yet again to do this in an easier fashion. I spoke with Loic about his concern, however, in this case, this is a feature we want in and supports many customers.

Fryguy · 2018-08-06T18:26:32Z

app/controllers/ops_controller/settings/common.rb

@@ -1104,11 +1106,16 @@ def settings_set_form_vars_logos
    if @edit[:current].config[:server][:custom_login_logo].nil?
      @edit[:current].config[:server][:custom_login_logo] = false # Set default custom_logo flag
    end
+    if @edit[:current].config[:server][:custom_brand].nil?
+      @edit[:current].config[:server][:custom_brand] = false # Set default custom_logo flag


Comment is wrong. https://twitter.com/nzkoz/status/538892801941848064

Fryguy · 2018-08-06T18:29:49Z

app/controllers/ops_controller/settings/upload.rb

-        File.open(typ == "custom" ? @@logo_file : @@login_logo_file, "wb") { |f| f.write(fld[:logo].read) }
-        msg = typ == "custom" ? _('Custom logo file "%{name}" uploaded') : _('Custom login file "%{name}" uploaded')
-        add_flash(msg % {:name => fld[:logo].original_filename})
+        File.open(file, "wb") { |f| f.write(field[:logo].read) }


A little simpler File.write(field[:logo].read, :mode => "wb")

This doesn't really make sense? Should I put this into the block, or replace the whole line? I haven't found any documentation for this.

Fryguy · 2018-08-06T18:37:29Z

app/controllers/ops_controller/settings/upload.rb

+  private
+
+  def logo_dir
+    dir = File.expand_path(Rails.root.join('public', 'upload'))


You can use the Pathname object directly instead of relying on the File class methods

dir = Rails.root.join('public', 'upload').expand_path Dir.mkdir(dir) unless dir.exist? dir.to_s

Fryguy · 2018-08-06T18:39:01Z

@skateman Where is the code that sets the custom_login setting in the settings (just want to make sure it's being done accurately.

terezanovotna · 2018-08-07T07:30:34Z

From what I see, LGTM! It should be ok to remove RH branding and replace it with another. We should then say more about RH branding within the About Modal

skateman · 2018-08-07T08:35:32Z

@Fryguy I don't understand the question, everything should be in the files I touched.

Rohoover · 2018-08-07T12:09:55Z

There is some valuable information here on cobranding:

https://brand.redhat.com/logos/cobranding/

That said, this isn't quite the same case (RH Logo + Partner logo on the same page). I would continue to do what was previously done, unless it directly runs counter to what's outlined in the link above.

skateman · 2018-08-13T14:20:22Z

@miq-bot rm_label pending core

epwinchell · 2018-08-13T14:34:17Z

@miq-bot add_label enhancement, gaprindashvili/yes

epwinchell · 2018-08-13T14:40:11Z

@miq-bot add_label fine/yes

mzazrivec · 2018-09-04T09:08:43Z

app/views/ops/_settings_custom_logos_tab.html.haml

+        %label.col-md-2.control-label
+          = _("Use Custom Brand Image")
+        .col-md-8
+          = check_box_tag("server_usebrand", true, @edit[:new][:server][:custom_brand], :data => {:on_text => 'Yes', :off_text => 'No'})


Missing gettext in {:on_text => 'Yes', :off_text => 'No'}

mzazrivec · 2018-09-05T16:17:34Z

app/controllers/ops_controller/settings/upload.rb

-        msg = typ == "custom" ? _('Custom logo file "%{name}" uploaded') : _('Custom login file "%{name}" uploaded')
-        add_flash(msg % {:name => fld[:logo].original_filename})
+        File.open(file, "wb") { |f| f.write(field[:logo].read) }
+        add_flash('%{image} "%{name}" uploaded' % {:image => text, :name => field[:logo].original_filename})


Missing gettext.

mzazrivec · 2018-09-06T10:37:26Z

app/controllers/ops_controller/settings/upload.rb

-        msg = typ == "custom" ? _('Custom logo file "%{name}" uploaded') : _('Custom login file "%{name}" uploaded')
-        add_flash(msg % {:name => fld[:logo].original_filename})
+        File.open(file, "wb") { |f| f.write(field[:logo].read) }
+        add_flash(_('%{image} "%{name}" uploaded') % {:image => text, :name => field[:logo].original_filename})


%{} won't work in a '...' string, right? You have to use "..." here.

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

miq-bot · 2018-09-06T11:03:56Z

Checked commits skateman/manageiq-ui-classic@712c431~...da1cfb5 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
6 files checked, 2 offenses detected

**

💣 💥 🔥 🚒 - Linter/Haml - Linter::Haml STDERR:

warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

app/controllers/ops_controller/settings/common.rb

❗ - Line 9, Col 3 - Style/ClassVars - Replace class var @@login_brand_file with a class instance var.

skateman commented Jul 30, 2018

View reviewed changes

miq-bot requested review from epwinchell and martinpovolny July 30, 2018 11:27

miq-bot added pending core wip labels Jul 30, 2018

skateman force-pushed the product-title branch from d1b2e75 to e3f2bde Compare July 30, 2018 12:53

miq-bot requested a review from dclarizio July 30, 2018 13:50

skateman changed the title ~~[WIP] Allow to upload and display a custom brand image on login/header~~ [WIP] [RFE] Allow to upload and display a custom brand image on login/header Jul 30, 2018

skateman force-pushed the product-title branch 2 times, most recently from d227c3b to 130c2ba Compare July 31, 2018 16:27

dclarizio requested review from h-kataria and removed request for dclarizio July 31, 2018 20:58

skateman force-pushed the product-title branch from 130c2ba to 03f9deb Compare August 1, 2018 15:15

skateman changed the title ~~[WIP] [RFE] Allow to upload and display a custom brand image on login/header~~ [RFE] Allow to upload and display a custom brand image on login/header Aug 1, 2018

miq-bot removed the wip label Aug 1, 2018

epwinchell approved these changes Aug 1, 2018

View reviewed changes

h-kataria approved these changes Aug 2, 2018

View reviewed changes

Fryguy reviewed Aug 6, 2018

View reviewed changes

skateman force-pushed the product-title branch from 03f9deb to 9ebc62c Compare August 7, 2018 08:34

miq-bot removed the pending core label Aug 13, 2018

miq-bot added enhancement gaprindashvili/yes labels Aug 13, 2018

miq-bot added the fine/yes label Aug 13, 2018

mzazrivec reviewed Sep 4, 2018

View reviewed changes

skateman force-pushed the product-title branch from 9ebc62c to 6ebe4e8 Compare September 4, 2018 11:01

mzazrivec self-assigned this Sep 5, 2018

mzazrivec reviewed Sep 5, 2018

View reviewed changes

skateman force-pushed the product-title branch from 6ebe4e8 to af86d5e Compare September 6, 2018 08:34

mzazrivec reviewed Sep 6, 2018

View reviewed changes

skateman added 2 commits September 6, 2018 13:01

Refactored custom logo uploading methods for better readability

712c431

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

Allow to upload and display a custom brand image on login/header

da1cfb5

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

skateman force-pushed the product-title branch from af86d5e to da1cfb5 Compare September 6, 2018 11:01

mzazrivec added this to the Sprint 94 Ending Sep 10, 2018 milestone Sep 6, 2018

mzazrivec merged commit ab0c8e5 into ManageIQ:master Sep 6, 2018

skateman deleted the product-title branch September 6, 2018 12:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFE] Allow to upload and display a custom brand image on login/header #4376

[RFE] Allow to upload and display a custom brand image on login/header #4376

skateman commented Jul 30, 2018 •

edited

skateman Jul 30, 2018

epwinchell commented Jul 30, 2018

h-kataria commented Aug 1, 2018

epwinchell commented Aug 1, 2018

Loicavenel commented Aug 1, 2018

epwinchell commented Aug 1, 2018

Loicavenel commented Aug 1, 2018

epwinchell commented Aug 1, 2018 •

edited

Loicavenel commented Aug 2, 2018

dclarizio commented Aug 2, 2018

bascar commented Aug 2, 2018

Fryguy Aug 6, 2018

Fryguy Aug 6, 2018 •

edited

skateman Aug 7, 2018

Fryguy Aug 6, 2018

Fryguy commented Aug 6, 2018

terezanovotna commented Aug 7, 2018

skateman commented Aug 7, 2018

Rohoover commented Aug 7, 2018

skateman commented Aug 13, 2018

epwinchell commented Aug 13, 2018

epwinchell commented Aug 13, 2018

mzazrivec Sep 4, 2018

mzazrivec Sep 5, 2018

mzazrivec Sep 6, 2018

miq-bot commented Sep 6, 2018

[RFE] Allow to upload and display a custom brand image on login/header #4376

[RFE] Allow to upload and display a custom brand image on login/header #4376

Conversation

skateman commented Jul 30, 2018 • edited

skateman Jul 30, 2018

Choose a reason for hiding this comment

epwinchell commented Jul 30, 2018

h-kataria commented Aug 1, 2018

epwinchell commented Aug 1, 2018

Loicavenel commented Aug 1, 2018

epwinchell commented Aug 1, 2018

Loicavenel commented Aug 1, 2018

epwinchell commented Aug 1, 2018 • edited

Loicavenel commented Aug 2, 2018

dclarizio commented Aug 2, 2018

bascar commented Aug 2, 2018

Fryguy Aug 6, 2018

Choose a reason for hiding this comment

Fryguy Aug 6, 2018 • edited

Choose a reason for hiding this comment

skateman Aug 7, 2018

Choose a reason for hiding this comment

Fryguy Aug 6, 2018

Choose a reason for hiding this comment

Fryguy commented Aug 6, 2018

terezanovotna commented Aug 7, 2018

skateman commented Aug 7, 2018

Rohoover commented Aug 7, 2018

skateman commented Aug 13, 2018

epwinchell commented Aug 13, 2018

epwinchell commented Aug 13, 2018

mzazrivec Sep 4, 2018

Choose a reason for hiding this comment

mzazrivec Sep 5, 2018

Choose a reason for hiding this comment

mzazrivec Sep 6, 2018

Choose a reason for hiding this comment

miq-bot commented Sep 6, 2018

skateman commented Jul 30, 2018 •

edited

epwinchell commented Aug 1, 2018 •

edited

Fryguy Aug 6, 2018 •

edited