Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Channel Icon Updates #437

Merged
merged 2 commits into from
Sep 13, 2018
Merged

Channel Icon Updates #437

merged 2 commits into from
Sep 13, 2018

Conversation

rossmoody
Copy link
Contributor

@rossmoody rossmoody commented Sep 12, 2018 •
edited
Loading

Fixes brave/brave-browser#1101

  • Updated the channel app icons to the appropriate sizes, colors and labels for the build channel they are associated with for consistency moving forward
  • Removed the public facing application name hyphens in favor of a consistent naming structure convention - i.e. "Brave Browser Channel Name"
  • "Nightly" icon received a color change to help distinguish it from the Dev icon
  • "Test" channel icon received a label to differentiate it from the "Development" channel.
  • Typeface updated from helvetica to poppins

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Needed or QA/No-QA-Needed) to include the closed issue in milestone

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

@rossmoody
Channel Icon Updates
7f75322 
-updated the channel app icons to the appropriate sizes, colors and labels for the build channel they are associated with for consistency moving forward
- removed the public facing application name hyphens in favor of a consistent naming structure without 'Browser"
- "Nightly" icon received a color change to help distinguish it from the Dev icon
- "Test" channel icon received a label to differentiate it from the "Development" channel.
@rossmoody rossmoody self-assigned this Sep 12, 2018
@rossmoody rossmoody added the UI label Sep 12, 2018
bbondy
bbondy reviewed Sep 12, 2018
View reviewed changes
app/theme/brave/BRANDING Outdated
@@ -1,6 +1,6 @@
COMPANY_FULLNAME=Brave Software, Inc.
COMPANY_SHORTNAME=Brave Software
PRODUCT_FULLNAME=Brave-Browser
PRODUCT_FULLNAME=Brave
Copy link
Member

Choose a reason for hiding this comment

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

nit: trailing space

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this will have extra consequences, but we want to be able to install Brave and Brave-Browser side by side. cc @simonhong

Copy link
Member

Choose a reason for hiding this comment

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

I think for this to be accepted, at least in part faster, pls split out the icon updates from the branding name changes.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this will have extra consequences, but we want to be able to install Brave and Brave-Browser side by side.

This change makes stable version as Brave in launchpad.
If user has muon, two Brave will be shown in launch pad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I didn't take that into consideration. Could we compromise on just removing the hyphen but keeping Browser in these? My hope was just to omit the hyphens for the display names to make the app feel a little more polished. So this would be "Brave Browser", "Brave Browser Dev", etc...

@rossmoody
Product Fullname Adjustments
71078e5 
- added "Browser" back into the naming convention for the channel product fullnames
@bbondy
Copy link
Member

bbondy commented Sep 12, 2018

I think that sounds good. cc @simonhong to review.

@bsclifton
Copy link
Member

@rossmoody can you share some screenshots? (ex: before and after)

@rossmoody
Copy link
Contributor Author

In that issue above I tried to outline some before and after graphics and talk a bit more in depth about the proposed changes. There aren't too many and the latest commit reinstated the "Browser" in the app names (which contradicts the screenshot).

@simonhong
Copy link
Member

simonhong commented Sep 13, 2018 •
edited
Loading

Blocked on brave/brave-browser#1106 and brave/brave-browser#1109

When fullname is changed, user data dir is also affected on MacOS. also start.js script should be modified.
I'm working this now.

@bbondy User data data folder should not be changed by this change. right?

This was referenced Sep 13, 2018
Merged
Closed
@bbondy
Copy link
Member

bbondy commented Sep 13, 2018

We can't change dev channel, but we can change for beta and release

@bbondy
Copy link
Member

bbondy commented Sep 13, 2018

Unless we patch to support spaces -> dashes for the user data dir. Seems like there might be other bugs introduced though.

@bbondy
Copy link
Member

bbondy commented Sep 13, 2018

oh n/m I seen you already have a PR for that 👍

@simonhong simonhong mentioned this pull request Sep 13, 2018
Merged
16 tasks
bbondy
bbondy approved these changes Sep 13, 2018
View reviewed changes
Copy link
Member

@bbondy bbondy left a comment

Choose a reason for hiding this comment

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

Issue icons look great!

@bbondy bbondy merged commit 99604bb into master Sep 13, 2018
bbondy added a commit that referenced this pull request Sep 13, 2018
@bbondy
Merge pull request #437 from brave/app-icon-updates
37faaa4 
Channel Icon Updates
@bbondy
Copy link
Member

bbondy commented Sep 13, 2018

master 99604bb
0.55.x 37faaa4

@rossmoody
Copy link
Contributor Author

So so so pumped about this! Go team!

darkdh added a commit that referenced this pull request Sep 14, 2018
@darkdh
Update brading unittest
ce8def7 
changed in #437

Auditor: @bbondy, @rossmoody
@darkdh darkdh mentioned this pull request Sep 14, 2018
Merged
11 tasks
@mbacchi mbacchi mentioned this pull request Sep 14, 2018
Closed
@LaurenWags LaurenWags mentioned this pull request Sep 20, 2018
Closed
@rossmoody rossmoody mentioned this pull request Sep 21, 2018
Merged
@bsclifton bsclifton deleted the app-icon-updates branch September 26, 2018 05:44
@bbondy bbondy added this to the 0.55.x - Release milestone Jan 14, 2019
cezaraugusto pushed a commit that referenced this pull request Jul 17, 2019
@ryanml
Imported from brave-ui: Merge pull request #437 from brave/console-fix
37b96f2 
Fixing panel console error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonhong simonhong simonhong left review comments

@bbondy bbondy bbondy approved these changes

@bsclifton bsclifton Awaiting requested review from bsclifton

@petemill petemill Awaiting requested review from petemill

@bradleyrichter bradleyrichter Awaiting requested review from bradleyrichter

Assignees

@rossmoody rossmoody

Labels
UI
Projects
None yet
Milestone
0.55.x - Release
Development

Successfully merging this pull request may close these issues.

Align App Channel Iconography and Naming Convention
4 participants
@rossmoody @bbondy @bsclifton @simonhong