-
Notifications
You must be signed in to change notification settings - Fork 113
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r+ with nits
|
||
// Note that injecting snippets can throw if they're invalid XML. | ||
if (containerEl) { | ||
containerEl.style.display = "block"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unhide this after
@@ -0,0 +1,261 @@ | |||
const DATABASE_NAME = "abouthome"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe rename to aboutnewtab?
@@ -8,6 +8,10 @@ | |||
</head> | |||
<body class="activity-stream"> | |||
<div id="root"></div> | |||
<div id="topSection"></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just add a comment here about why it's needed, since this sounds important 😅
system-addon/lib/SnippetsFeed.jsm
Outdated
} | ||
onAction(action) { | ||
switch (action.type) { | ||
case at.INIT: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add feed init case here
This adds an initial implementation of snippets integration. It currently works with the v4 snippets endpoint (although it looks a bit weird). Remaining issues:
Closes #2013 and #2837