Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Change facebook top tile to github and improve styling of top tiles icons #14073

Merged
merged 2 commits into from
May 14, 2018
Merged

Change facebook top tile to github and improve styling of top tiles icons #14073

merged 2 commits into from
May 14, 2018

Conversation

Jacalz
Copy link
Contributor

@Jacalz Jacalz commented May 9, 2018

Fixes #14072

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.

@Jacalz Jacalz self-assigned this May 9, 2018
bradleyrichter
bradleyrichter previously approved these changes May 9, 2018
Copy link
Contributor

@bradleyrichter bradleyrichter left a comment

Choose a reason for hiding this comment

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

perfect solution. Approved!

@NejcZdovc NejcZdovc added this to the Completed work milestone May 9, 2018
@bsclifton bsclifton modified the milestones: Completed work, 0.24.x (Nightly Channel) May 14, 2018
'favicon': `${iconPath}/github.png`,
'location': 'https://github.com/brave/',
'themeColor': 'rgb(255, 255, 255)',
'title': 'Brave Software | Github'
Copy link
Member

Choose a reason for hiding this comment

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

this should be GitHub

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is fixed now 👍

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Comment left about casing on new top site tile 😄 Otherwise, looks great!

@Jacalz
Copy link
Contributor Author

Jacalz commented May 14, 2018

The fix have been pushed and squashed now @bsclifton 🙂

@@ -63,6 +63,6 @@ module.exports.topSites = [
'favicon': `${iconPath}/playstore.png`,
'location': 'https://play.google.com/store/apps/details?id=com.brave.browser',
'themeColor': 'rgb(241, 241, 241)',
'title': 'Brave Browser: Fast AdBlock – Apps para Android no Google Play'
'title': 'Brave Browser: Fast AdBlock – Apps for Android not Google Play'
Copy link
Contributor

@NejcZdovc NejcZdovc May 14, 2018

Choose a reason for hiding this comment

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

If I go to the link above it says Brave Browser: Fast AdBlocker - Apps on Google Play. I think we need to update it

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed now with latest commit 👍

@Jacalz
Copy link
Contributor Author

Jacalz commented May 14, 2018

I noticed that the playstore logo was outdated, so I updated it also 👍
image

@Jacalz Jacalz requested review from bradleyrichter and removed request for bradleyrichter May 14, 2018 15:30
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

++

@bsclifton bsclifton merged commit 0afa631 into brave:master May 14, 2018
@Jacalz
Copy link
Contributor Author

Jacalz commented May 14, 2018

Thanks for merging 🙏

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.

4 participants