Skip to content
This repository has been archived by the owner on Feb 29, 2020. It is now read-only.

Add TippyTop sites to the System Addon #2944

Closed
jaredlockhart opened this issue Jul 24, 2017 · 5 comments
Closed

Add TippyTop sites to the System Addon #2944

jaredlockhart opened this issue Jul 24, 2017 · 5 comments

Comments

@jaredlockhart
Copy link
Contributor

jaredlockhart commented Jul 24, 2017

Also see https://bugzilla.mozilla.org/show_bug.cgi?id=1382703
Import the Tippy Top sites list and icons into the System Addon and use them for matching URLs.

@jaredlockhart
Copy link
Contributor Author

jaredlockhart commented Jul 24, 2017

We need to figure out whether we will bundle the icons directly with the browser or fetch them from a remote CDN.

See legal: https://bugzilla.mozilla.org/show_bug.cgi?id=1374898

@jaredlockhart jaredlockhart self-assigned this Jul 24, 2017
@jaredlockhart
Copy link
Contributor Author

jaredlockhart commented Jul 24, 2017

  1. pull the npm package (already defined in package.json) https://github.com/mozilla/tippy-top-sites/blob/master/top_sites.json
  2. in yamlscripts.yml define a script to copy it from node_modules into a static asset path inside the system addon like data/tippytopicons https://github.com/mozilla/activity-stream/blob/master/yamscripts.yml#L15
  3. then buildmc kicks in and copies the whole system addon into the mc checkout, so now you can find the json manifest and all the pngs locally
  4. load the manifest json at activity stream init into memory inside a tippy top helper
  5. in TopSitesFeed when looping over topsite links, check if they appear in the TippyTop provider and have an icon https://github.com/mozilla/activity-stream/blob/master/system-addon/lib/TopSitesFeed.jsm#L74-L77
  6. if it has it, attach it, and now you don’t need to fetch a screenshot
  7. in content, check for tippy top icon and render that otherwise check for screenshot and render that https://github.com/mozilla/activity-stream/blob/master/system-addon/content-src/components/TopSites/TopSites.jsx#L43-L50

@jaredlockhart jaredlockhart assigned rlr and unassigned jaredlockhart Jul 24, 2017
@jaredlockhart
Copy link
Contributor Author

I think we'll proceed with the assumption that we can do it the same way that we did in Test Pilot and if we have to change this to fetch from a remote resource we can do that.

rlr added a commit to rlr/activity-streams that referenced this issue Jul 25, 2017
@Mardak Mardak removed the Blocked label Jul 26, 2017
@Mardak
Copy link
Member

Mardak commented Jul 26, 2017

For 56 shield study, we need to just have tippy top for the default sites. Locale list is unsure yet but it will be at least en-US. Maybe de?

@Mardak
Copy link
Member

Mardak commented Jul 27, 2017

The tippy top sites should match those in #2776 (comment)

So the related US, Canada, Germany, Poland, UK, France, Russia sites.

rlr added a commit to rlr/activity-streams that referenced this issue Aug 1, 2017
rlr added a commit to rlr/activity-streams that referenced this issue Aug 1, 2017
rlr added a commit to rlr/activity-streams that referenced this issue Aug 1, 2017
rlr added a commit to rlr/activity-streams that referenced this issue Aug 1, 2017
rlr added a commit to rlr/activity-streams that referenced this issue Aug 1, 2017
@rlr rlr closed this as completed in #3049 Aug 1, 2017
rlr added a commit that referenced this issue Aug 1, 2017
feat (systemaddon): #2944 add tippy top icons
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants