Skip to content
This repository has been archived by the owner on Jan 12, 2023. It is now read-only.

Add upstream and downstream favicons, and make webpack output more readable #373

Merged
1 commit merged into from Jan 13, 2021

Conversation

mturley
Copy link
Collaborator

@mturley mturley commented Jan 13, 2021

Resolves #357 (https://bugzilla.redhat.com/show_bug.cgi?id=1914741)

Adds favicon files for both Konveyor and Red Hat brands, and configures webpack to use the correct one depending on the BRAND_TYPE environment variable (falling back on Konveyor if that variable is undefined).

As part of this PR, I was frustrated trying to figure out why my env variable wasn't being passed correctly, because our webpack-dev-server progress output is so noisy that it's impossible to read the top of the output. I decided to disable the progress output of webpack to prevent this issue (no more percent progress, but we still get "waiting", "compiled", and "compiled successfully" messages). This keeps output light and readable, so the environment variables can easily be seen when running webpack.

If this issue is ever resolved: https://github.com/kimmobrunfeldt/concurrently/issues/85 we can turn progress back on because it supposed to be outputting to one line that gets updated instead of hundreds of lines.

@mturley mturley requested a review from a team January 13, 2021 18:38
@konveyor-preview-bot
Copy link

🚀 Deployed Preview: http://konveyor-forklift-ui-pr-373-preview.surge.sh

Compare with current master: http://konveyor-forklift-ui-preview.surge.sh

Base automatically changed from master to main January 13, 2021 18:45
@konveyor-preview-bot
Copy link

🚀 Deployed Preview: http://konveyor-forklift-ui-pr-373-preview.surge.sh

Compare with current main branch: http://konveyor-forklift-ui-preview.surge.sh

@codecov
Copy link

codecov bot commented Jan 13, 2021

Codecov Report

Merging #373 (58105c4) into main (5c8ffbe) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #373   +/-   ##
=======================================
  Coverage   62.84%   62.84%           
=======================================
  Files         121      121           
  Lines        3488     3488           
  Branches      797      797           
=======================================
  Hits         2192     2192           
  Misses       1279     1279           
  Partials       17       17           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c8ffbe...58105c4. Read the comment docs.

__dirname,
process.env.BRAND_TYPE === 'RedHat'
? '../src/favicon-redhat.ico'
: '../src/favicon-konveyor.ico'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mturley
I noticed in the sprint 195 demo that you are using the Konveyor icon rather than the Forklift icon. Will the Konveyor icon be used for all the UIs?

Copy link
Collaborator Author

@mturley mturley Jan 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apinnick I was only replacing Red Hat branding with Konveyor branding in the upstream, which I figured was equivalent because in the upstream builds we're using Red Hat icons (and not Migration Toolkit icons). It's a good question though, maybe we should be using product-specific favicons.

It's my understanding that MTC works the same way right now with Red Hat logos/icons and not Migration Toolkit ones, but I'm not sure if they have configured Konveyor ones in upstream builds yet.

Either way though the product logo could probably go on the welcome page instead of the generic migration icon.
@vconzola do you have thoughts on these, or know who we should ask?

@mturley mturley deleted the 357-favicon branch January 18, 2021 14:45
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BZ#1914741] Add a favicon Webpack progress output
3 participants