Fix Bug 1422079 - sections context menu #3987

rlr · 2018-02-19T22:59:43Z

Most of the +green+ is offset by -red- due to the removal of the infoOption <Info/> component stuff. yay

Remove <Info/> component and related tests, styles, etc.
Implement new <SectionMenu/> component which is a ContextMenu specific for sections.
** It is missing the "Move Up" and "Move Down" options until we implement reordering
** Some icons are still being manufactured @bryanbell

Screenshots:

rlr · 2018-02-20T13:58:20Z

@csadilek want to take a look at this PR too? specifically the Section related changes.

Mardak · 2018-02-20T21:36:31Z

@aaronrbenson @bryanbell Should all the menu options be actions, e.g., "Learn More" vs "About Pocket" and "View Privacy Notice" vs "Privacy Notice"?

Also, we don't call the top stories section "Pocket" right now… It's "Recommended by Pocket", so should the menu actions be "Manage Recommended by Pocket"?

aaronrbenson · 2018-02-20T23:43:41Z

Much of this is about brevity. See the attached example. I don't think the "About Pocket" link needs to necessarily be an action.

Mardak · 2018-02-21T01:22:45Z

system-addon/content-src/components/CollapsibleSection/CollapsibleSection.jsx

@@ -135,12 +51,10 @@ export class _CollapsibleSection extends React.PureComponent {
  constructor(props) {
    super(props);
    this.onBodyMount = this.onBodyMount.bind(this);
-    this.onInfoEnter = this.onInfoEnter.bind(this);
-    this.onInfoLeave = this.onInfoLeave.bind(this);


Were these not actually used by CollapsibleSection? Looks like it was added twice but only needed in Info?

oh weird I didnt even notice that! I guess it was some copy/pasta fail when creating <Disclaimer/>

oh, I guess this isn't <Disclaimer/>. I fail at reading diffs. Yeah, this must've been a fail when I broke <Info/> into it's own component or something.

Mardak

Looks to be working although @csadilek might be able to point to some better reuse of available data in the top story options instead of redefining them.

Mardak · 2018-02-21T14:28:49Z

locales/en-US/strings.properties

@@ -193,3 +198,15 @@ error_fallback_default_info=Oops, something went wrong loading this content.
 # that failed to render, and suggests the likely way to deal with the failure:
 # reloading the page.
 error_fallback_default_refresh_suggestion=Refresh page to try again.
+


The previous commit removing info stuff should have a commit message indicating which section_info, etc strings are unused and reference bug 1433209, e.g., https://bugzilla.mozilla.org/show_bug.cgi?id=1433209#c1

Mardak · 2018-02-21T14:28:49Z

locales/en-US/strings.properties

+menu_action_expand_section=Expand Section
+menu_action_manage_section=Manage {section}
+menu_action_add_topsite=Add Top Site
+menu_action_about_pocket=About Pocket


We probably want this to be "About {section}" for web extension support depending on if we should be triggering this based on the existence of a learn-more type link in the section object? @csadilek ?

Also, we want a localization note about the {section} usage.

@Mardak yes, agreed on keeping this generic, if possible. menu_action_about_section = About {section}.

Design feedback is that this is specifically the privacy notice link, so just make it "Privacy Notice" and point to the appropriate url from the option.

Mardak · 2018-02-21T14:28:49Z

locales/en-US/strings.properties

@@ -193,3 +198,15 @@ error_fallback_default_info=Oops, something went wrong loading this content.
 # that failed to render, and suggests the likely way to deal with the failure:
 # reloading the page.
 error_fallback_default_refresh_suggestion=Refresh page to try again.
+
+# LOCALIZATION NOTE (menu_action_*).  These strings are displayed in the section
+# context menu and are meant as a call to action for the given section.


There's already menu_action_* for card context menu stuff. Not sure if we should name these section_menu_* e.g., section_menu_remove_section @flodolo ?

section_menu_* sounds good

Mardak · 2018-02-21T14:28:49Z

locales/en-US/strings.properties

+menu_action_about_pocket=About Pocket
+menu_action_move_up=Move Up
+menu_action_move_down=Move Down
+menu_action_privacy_notice=Privacy Notice


We already have this as section_info_privacy_notice=Privacy Notice although it's used as a link instead of a menu option. @flodolo ? (The "section_info_*" will be unused with this change..)

Since you plan to remove the other one, it wouldn't hurt to have this string added.

Mardak · 2018-02-21T14:28:49Z

system-addon/content-src/components/CollapsibleSection/_CollapsibleSection.scss

@@ -16,6 +19,50 @@

  .section-top-bar {
    position: relative;
+
+    .context-menu-button {
+      background: url('chrome://browser/skin/page-action.svg') no-repeat right center;


Can we share some of this into _icons.scss with the other context menu button?

Mardak · 2018-02-21T14:28:49Z

system-addon/content-src/components/Sections/Sections.jsx

-      <CollapsibleSection className="section" icon={icon} title={getFormattedMessage(title)}
+      <CollapsibleSection className="section" icon={icon}
+        title={this.getFormattedMessage(title)}
+        name={id === "topstories" ? "Pocket" : this.getFormattedMessage(title)}


We should have the provider_name somewhere as we were using it for "Recommended by {provider}"... I guess it's part of the title object?

Mardak · 2018-02-21T14:28:49Z

system-addon/content-src/components/Sections/Sections.jsx

@@ -30,6 +26,10 @@ export class Section extends React.PureComponent {
    }
  }

+  getFormattedMessage(message) {
+    return typeof message === "string" ? <span>{message}</span> : this.props.intl.formatMessage({id: message.id}, message.values);


This doesn't look quite right. If we pass in a "string", we get html/jsx, but if it's an "object," we get a string? Maybe it should always return a string?

I can undo these changes now that we will just have the link be "Manage Section".

Mardak · 2018-02-21T14:28:49Z

system-addon/content-src/components/CollapsibleSection/CollapsibleSection.jsx

+            </button>
+            <SectionMenu
+              options={menuOptions}
+              name={name}


Maybe instead of CollapsibleSection prop name there's an optional shortName where if there is no shortName, title is used. E.g., sectionName={shortName || title} ? (And have SectionMenu prop name renamed to sectionName)

Mardak · 2018-02-21T14:28:49Z

system-addon/content-src/lib/section-menu-options.js

+  AboutPocket: section => ({
+    id: "menu_action_about_pocket",
+    icon: "about",
+    redirectUrl: "https://getpocket.cdn.mozilla.net/firefox/new_tab_learn_more",


We should pass in the disclaimer_link as part of the section to use here.

Could we change this to AboutSection instead? Section could hold an object that further describes this i.e. section.about? SectionManager can provide this about object based on topstories.options.

This just in from the design meeting. This should be "Privacy Notice" and link to https://www.mozilla.org/en-US/privacy/firefox/#suggest-relevant-content which is info_link but needs the #anchor updated.

cool. So we'll have a PrivacyNotice option and no About* section and just conditionally show it based on if it has the url in the section options.

Mardak · 2018-02-21T14:28:49Z

system-addon/lib/SectionsManager.jsm

@@ -41,7 +41,8 @@ const BUILT_IN_SECTIONS = {
      button: {id: options.disclaimer_buttontext || "section_disclaimer_topstories_buttontext"}
    },
    maxRows: 1,
-    availableContextMenuOptions: ["CheckBookmark", "SaveToPocket", "Separator", "OpenInNewWindow", "OpenInPrivateWindow", "Separator", "BlockUrl"],
+    sectionMenuOptions: ["RemoveSection", "CheckCollapsed", "Separator", "AboutPocket", "ManageSection"],


I wonder if we want to have a common list where "About" gets added in dynamically?

I guess we could have a "CheckAbout". I'll try that.

flodolo · 2018-02-21T14:39:15Z

locales/en-US/strings.properties

+# LOCALIZATION NOTE(section_context_menu_button_sr): This is for screen readers when
+# the section edit context menu button is focused/active. Title is the name of
+# the section (Top Sites, Highlights, etc).
+section_context_menu_button_sr=Open context menu for {title} section


This might not work with all languages, because you're reusing a stand-alone string (title) in the middle of another one. Not sure if there's a better way to do it, besides removing the {title} completely.

flodolo · 2018-02-21T14:39:15Z

locales/en-US/strings.properties

@@ -193,3 +198,15 @@ error_fallback_default_info=Oops, something went wrong loading this content.
 # that failed to render, and suggests the likely way to deal with the failure:
 # reloading the page.
 error_fallback_default_refresh_suggestion=Refresh page to try again.
+
+# LOCALIZATION NOTE (menu_action_*).  These strings are displayed in the section
+# context menu and are meant as a call to action for the given section.


section_menu_* sounds good

flodolo · 2018-02-21T14:39:15Z

locales/en-US/strings.properties

+menu_action_remove_section=Remove Section
+menu_action_collapse_section=Collapse Section
+menu_action_expand_section=Expand Section
+menu_action_manage_section=Manage {section}


Same note about {section}, you'd be using a string (name of section) in the middle of a sentence, with potential problems (case, grammatical case and declension). Does it need to be there?

Design feedback was to just switch to "Manage Section"

I like 👍 😄

flodolo · 2018-02-21T14:39:15Z

locales/en-US/strings.properties

+menu_action_about_pocket=About Pocket
+menu_action_move_up=Move Up
+menu_action_move_down=Move Down
+menu_action_privacy_notice=Privacy Notice


Since you plan to remove the other one, it wouldn't hurt to have this string added.

csadilek · 2018-02-21T16:14:02Z

system-addon/lib/SectionsManager.jsm

-      header: {id: options.provider_header || "pocket_feedback_header"},
-      body: {id: options.provider_description || "pocket_feedback_body"},
-      link: {href: options.info_link, id: "section_info_privacy_notice"}
-    },


Here's where I thought we could configure the "about object" based on topstories.options, which we could then access in section-menu-options.js.

csadilek

@rlr @Mardak looks good to me.

2 minor things:

Suggested how to re-use topstories.options so we can have a generic AboutSection menu option, similar to what we had before where SectionManager defines an object based on .options. Happy to discuss alternative solutions as well.
With this change we can cleanup topstories.options and remove two fields (provider_header and info_link) as they are no longer used: https://github.com/mozilla/activity-stream/blob/master/system-addon/lib/ActivityStream.jsm#L54

Latest update: Seems info_link from topstories.options will still be used with updated anchor.

rlr · 2018-02-22T02:53:48Z

I think I addressed all the feedback so far. Icons still pending...

Mardak

Mainly questions about simplifying SectionMenu.jsx

Mardak · 2018-02-22T08:07:22Z

system-addon/lib/SectionsManager.jsm

@@ -40,13 +40,10 @@ const BUILT_IN_SECTIONS = {
      },
      button: {id: options.disclaimer_buttontext || "section_disclaimer_topstories_buttontext"}
    },
+    privacyNoticeURL: "https://www.mozilla.org/en-US/privacy/firefox/#suggest-relevant-content",


Hah sorry. https://www.mozilla.org/privacy/firefox/#suggest-relevant-content (let the server decide locale)

Also, I would have expected this to be in ActivityStream.jsm replacing the pref json entry for info_link? @csadilek mentioned a "topstories.options" in a previous comment.

(I believe the reasoning for the json pref instead of hardcoded in SectionManager is that if there were a different story provider, it wouldn't necessarily share the same values as Pocket……… although in this case, I suppose a generic Privacy Notice for any "suggested content" might be common for any top stories… ???)

@Mardak @rlr Yes, let's remove info_link from topstories.options and add privacy_link instead to make this configurable. We can also delete provider_header (from options) and the corresponding string pocket_feedback_header now that we're no longer using it.

If you prefer not having to configure this link we can add:
options.privacy_link || "https://www.mozilla.org/en-US/privacy/firefox/#suggest-relevant-content"

This way we can override in case we ever need to, but it will work by default.

makes sense. will fix. thanks!

Mardak · 2018-02-22T08:07:22Z

system-addon/content-src/components/SectionMenu/SectionMenu.jsx

+  getOptions() {
+    const {props} = this;
+
+    const propOptions = props.options || DEFAULT_SECTION_MENU_OPTIONS;


What are your thoughts on having a mostly common base options that don't need to even be passed? E.g.,

const propOptions = Array.from(DEFAULT_SECTION_MENU_OPTIONS); // Prepend custom options and a separator if (extraOptions) { propOptions.splice(0, 0, ...extraOptions, "Separator"); } // Insert privacy notice before the last option ("ManageSection") if (props.privacyNoticeUrl) { propOptions.splice(-1, 0, "PrivacyNotice"); }

Where only top sites passes in extraOptions and stories passes in the url.

I could go either way with this one. 🤔 👍 👎 ? coin toss?

I think the main thing is that DEFAULT_SECTION_MENU_OPTIONS is basically duplicated in 5 places right now, so avoiding bugs when accidentally changing a subset of them

That's a good reason 😄

Mardak · 2018-02-22T08:07:22Z

system-addon/content-src/components/SectionMenu/SectionMenu.jsx

+            }));
+          }
+          if (redirectUrl) {
+            document.location = redirectUrl;


The action check and this custom redirectUrl aren't necessary as there's an OPEN_LINK action to navigate.

Mardak · 2018-02-22T08:07:22Z

system-addon/content-src/components/SectionMenu/SectionMenu.jsx

+
+    const propOptions = props.options || DEFAULT_SECTION_MENU_OPTIONS;
+
+    const options = propOptions.map(o => SectionMenuOptions[o](props)).map(option => {


This would seem like it could be shared with LinkMenu.jsx's functionality as higher order function probably from a shared content-src/lib/menu-options, e.g., makeOptions(propOptions, SectionMenuOptions)? @sarracini

Although now I realize React is probably rerendering this component like crazy because there's all these dynamic function bindings for onClick even though it probably could just be defined once and refer to this to get at options?

I guess all of this HOF and more cleanup should be a separate bug. Would be interesting to see if this is actually has performance impacts given how many context menus we have now… ~25 menus * ~10 options -> 250 ContextMenuItem being rendered every time a prop updates?

Yeah this little chunk here could be refactored to a more general "menu options" instead of a link menu and a sections menu. But I think you're right @Mardak, we should evaluate the performance and do it as a follow up

One thing we can easily do is instead of having:

render() { return (<ContextMenu visible={this.props.visible} onUpdate={this.props.onUpdate} options={this.getOptions()} />); }

we should have:

render() { return this.props.visible && (<ContextMenu visible={this.props.visible} onUpdate={this.props.onUpdate} options={this.getOptions()} />); }

That we we render on demand. Should save all the cpu cycles and kittens

I think the "react way" is something like this anyway:

{this.props.visible && <Component /> }

Yeah. I played with it but it requires changes in <ContextMenu/> because it currently assumes that it will be mounted initially with visible=false. But should be easyish fix. Just need to do LinkMenu and SectionMenu changes together, so I'd rather do as followup instead of piling on unrelated LinkMenu changes here.

Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1440456

Mardak · 2018-02-22T08:07:22Z

system-addon/lib/SectionsManager.jsm

-    availableContextMenuOptions: ["CheckBookmark", "SaveToPocket", "Separator", "OpenInNewWindow", "OpenInPrivateWindow", "Separator", "BlockUrl"],
-    infoOption: {
-      header: {id: options.provider_header || "pocket_feedback_header"},
-      body: {id: options.provider_description || "pocket_feedback_body"},


These strings are now unused I believe (so add to the commit comment). Although _body is already missing… ???

I think I saw them used in the Preferences panel. I’ll double check.

I think "pocket_feedback_body" was a bug there. Probably was meant to be "pocket_description"? We use that in preferences. I'll fix it in SectionManager.jsx. But the pocket_feedback_header can die.

Mardak · 2018-02-22T08:29:58Z

Oh, I thought I commented about this earlier but maybe I forgot… Do we need to support the "open to the right" vs "open to the left when there's not enough space on the right"?

rlr · 2018-02-22T20:10:38Z

@Mardak Everything should be addressed except for #3987 (comment) and the unnecessary rendering of all the context menus on every page load. Icons are coming. I might start hacking on the perf/rendering issue and keep the changes separate.

rlr · 2018-02-22T21:18:17Z

@Mardak done ^^. One think I am not excited about is a bunch of props are passed to <CollapsibleSection/> just to be passed along to <SectionMenu/>. Makes me wonder if there is a better way to factor it so that they are more like siblings instead. On the other hand, it's nice that we only create SectionMenu from that one component instead of from each of the parent components. 6 one way, half a dozen the other.

Mardak · 2018-02-22T21:44:17Z

I believe the way to avoid pass-through props is having getChildContext(), although we haven't really used it ourselves so far… @k88hudson might know more about when to use or avoid context.

Mardak

Almost working except OPEN_LINK and minor cleanup

Mardak · 2018-02-22T23:16:14Z

system-addon/content-src/lib/section-menu-options.js

+  PrivacyNotice: section => ({
+    id: "section_menu_action_privacy_notice",
+    icon: "privacy",
+    action: {type: at.OPEN_LINK, data: {url: section.privacyNoticeURL}},


This wants an OnlyToMain (right now it sends only to itself/content, so it does nothing.)

oops. guess I didn't test that 🤦‍♂️

Mardak · 2018-02-22T23:16:14Z

system-addon/content-src/components/SectionMenu/SectionMenu.jsx

+      if (!type && id) {
+        option.label = props.intl.formatMessage({id});
+        option.onClick = () => {
+          if (action) {


All options have actions, so this check isn't necessary.

Mardak · 2018-02-22T23:16:14Z

system-addon/content-src/components/CollapsibleSection/CollapsibleSection.jsx

+            </button>
+            <SectionMenu
+              options={menuOptions}
+              name={name}


name is unused here too.

Oh odd. This is indeed outdated. I wonder what view github was showing me earlier…

Mardak · 2018-02-22T23:16:14Z

system-addon/content-src/components/TopSites/TopSites.jsx

+        className="top-sites"
+        icon="topsites"
+        title={props.intl.formatMessage({id: "header_top_sites"})}
+        name={props.intl.formatMessage({id: "header_top_sites"})}


name is unused here too.

Mardak · 2018-02-22T23:20:54Z

Oh, and this adds to the UTEventReporting errors:
Unknown event: ["activity_stream", "event", "SECTION_MENU_MANAGE"]

https://bugzilla.mozilla.org/show_bug.cgi?id=1438239

The following strings can be removed after Firefox 60: * section_info_option * section_info_privacy_notice * pocket_feedback_header (Ping Bug 1433209)

rlr · 2018-02-22T23:57:36Z

I squashed into two commits. Just waiting on the icons now (cough @bryanbell cough)

Mardak · 2018-02-23T00:02:46Z

We could probably land without icons for now to get strings exported sooner

bryanbell · 2018-02-23T01:28:13Z

Icons needed.zip

Here are the icons needed for this menu.

Mardak · 2018-02-23T01:40:36Z

Same icon for collapse and expand?

Mardak · 2018-02-23T01:43:38Z

There's an existing glyph-info-16.svg but it has a thinner border… ? https://searchfox.org/mozilla-central/source/browser/extensions/activity-stream/data/content/assets/glyph-info-16.svg

rlr · 2018-02-23T01:59:56Z

YAY! @bryanbell gave me an expand icon too. Also confirmed that we should use the new -information icon.

Mardak · 2018-02-23T02:28:19Z

system-addon/data/content/assets/glyph-information-16.svg

@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16"><path d="M8 16a8 8 0 1 1 8-8 8.009 8.009 0 0 1-8 8zM8 2a6 6 0 1 0 6 6 6.006 6.006 0 0 0-6-6z"/><path d="M8 7a1 1 0 0 0-1 1v3a1 1 0 0 0 2 0V8a1 1 0 0 0-1-1z"/><circle cx="8" cy="5" r="1.188"/></svg>


If the old one isn't used, let's just replace glyph-info-16.svg

Mardak · 2018-02-23T02:29:57Z

system-addon/data/content/assets/glyph-info-option-12.svg

@@ -1 +0,0 @@
-<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 12 12"><path fill="context-fill" d="M6 0a6 6 0 1 0 6 6 6 6 0 0 0-6-6zm.7 10.26a1.13 1.13 0 0 1-.78.28 1.13 1.13 0 0 1-.78-.28 1 1 0 0 1 0-1.42 1.13 1.13 0 0 1 .78-.28 1.13 1.13 0 0 1 .78.28 1 1 0 0 1 0 1.42zM8.55 5a3 3 0 0 1-.62.81l-.67.63a1.58 1.58 0 0 0-.4.57 2.24 2.24 0 0 0-.12.74H5.06a3.82 3.82 0 0 1 .19-1.35 2.11 2.11 0 0 1 .63-.86 4.17 4.17 0 0 0 .66-.67 1.09 1.09 0 0 0 .23-.67.73.73 0 0 0-.77-.86.71.71 0 0 0-.57.26 1.1 1.1 0 0 0-.23.7h-2A2.36 2.36 0 0 1 4 2.47a2.94 2.94 0 0 1 2-.65 3.06 3.06 0 0 1 2 .6 2.12 2.12 0 0 1 .72 1.72 2 2 0 0 1-.17.86z"/></svg>


Looks like all the existing icons have viewBox instead of width/height:
svgo --multipass --disable=removeViewBox --enable={removeDimensions,removeStyleElement,sortAttrs} *.svg

Mardak · 2018-02-23T03:13:19Z

system-addon/data/content/assets/glyph-info-16.svg

Oh I forgot to mention this, but it's easier to tell now via this diff. It's missing fill="context-fill"

ah, and only that one.

fill: red !important ;)

I suppose this would have missed the white fill too, but that's harder to glance quickly ;)

Mardak · 2018-02-23T03:21:21Z

system-addon/data/content/assets/glyph-info-16.svg

ack! holldd onnn.

I guess wrap the path and circle with a <g> ?

Mardak · 2018-02-23T03:47:19Z

system-addon/content-src/components/TopSites/TopSites.jsx

-          <div className="add-topsites-button">
-            <button
-              className="add"
-              title={this.props.intl.formatMessage({id: "edit_topsites_add_button_tooltip"})}


I guess that was short lived. string already unused!

Actually, this was only added in this 60 cycle? It should be safe to just remove now as we don't need to keep it longer for beta

oh right. that was at the work week which was 60 already. nice

and I guess "edit_topsites_add_button" can be removed in a few releases too

rlr · 2018-02-23T23:13:43Z

system-addon/content-src/components/CollapsibleSection/_CollapsibleSection.scss

-      margin-left: 7px;
-    }
+  @media (max-width: $break-point-widest + $card-width * 1.5) {
+    @include context-menu-open-left;


this isn't specific enough and is causing link menus to open to the left too.

dmose · 2018-02-28T16:53:56Z

The context API is changing in some upcoming version of React. It might make sense to wait for this to solve the "prop-drilling" problem.

rlr added the PR / Needs review label Feb 19, 2018

rlr assigned Mardak Feb 19, 2018

rlr requested a review from Mardak Feb 19, 2018

rlr force-pushed the rlr:bug1437129/section-context-menu branch 2 times, most recently from 59dde7a to 51bd5d3 Feb 19, 2018

rlr force-pushed the rlr:bug1437129/section-context-menu branch from 51bd5d3 to 034b51d Feb 20, 2018

Mardak reviewed Feb 21, 2018

View changes

Mardak suggested changes Feb 21, 2018

View changes

flodolo reviewed Feb 21, 2018

View changes

csadilek reviewed Feb 21, 2018

View changes

rlr force-pushed the rlr:bug1437129/section-context-menu branch 2 times, most recently from 61966d1 to a417c9a Feb 22, 2018

Mardak reviewed Feb 22, 2018

View changes

rlr force-pushed the rlr:bug1437129/section-context-menu branch from a417c9a to 7e2b8f9 Feb 22, 2018

Mardak approved these changes Feb 22, 2018

View changes

Mardak assigned rlr and unassigned Mardak Feb 22, 2018

Mardak added PR / R+ with nits and removed PR / Needs review labels Feb 22, 2018

feat (sections) - remove info option code

a9345db

The following strings can be removed after Firefox 60: * section_info_option * section_info_privacy_notice * pocket_feedback_header (Ping Bug 1433209)

rlr force-pushed the rlr:bug1437129/section-context-menu branch from dd2d9b1 to abe782c Feb 22, 2018

rlr force-pushed the rlr:bug1437129/section-context-menu branch from 144910a to dddbb23 Feb 23, 2018

Mardak reviewed Feb 23, 2018

View changes

Fix Bug 1422079 - sections context menu

e939451

rlr force-pushed the rlr:bug1437129/section-context-menu branch from e132009 to e939451 Feb 23, 2018

rlr merged commit f6dc30b into mozilla:master Feb 23, 2018
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

This was referenced Feb 23, 2018

Merge latest strings from activity-stream changeset f3e33da50e0bea2fad0a38a985c5eab1d98eb38a mozilla/activity-stream-l10n#11

Merged

fix(tests): Update mochitests for section context menus #3993

Merged

rlr deleted the rlr:bug1437129/section-context-menu branch Feb 23, 2018

rlr reviewed Feb 23, 2018

View changes

dmose added the pushed-to-pine label Mar 6, 2018


		const propOptions = props.options \|\| DEFAULT_SECTION_MENU_OPTIONS;

		const options = propOptions.map(o => SectionMenuOptions[o](props)).map(option => {

		@@ -0,0 +1 @@
		<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16"><path d="M8 16a8 8 0 1 1 8-8 8.009 8.009 0 0 1-8 8zM8 2a6 6 0 1 0 6 6 6.006 6.006 0 0 0-6-6z"/><path d="M8 7a1 1 0 0 0-1 1v3a1 1 0 0 0 2 0V8a1 1 0 0 0-1-1z"/><circle cx="8" cy="5" r="1.188"/></svg>

		@@ -1 +0,0 @@
		<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 12 12"><path fill="context-fill" d="M6 0a6 6 0 1 0 6 6 6 6 0 0 0-6-6zm.7 10.26a1.13 1.13 0 0 1-.78.28 1.13 1.13 0 0 1-.78-.28 1 1 0 0 1 0-1.42 1.13 1.13 0 0 1 .78-.28 1.13 1.13 0 0 1 .78.28 1 1 0 0 1 0 1.42zM8.55 5a3 3 0 0 1-.62.81l-.67.63a1.58 1.58 0 0 0-.4.57 2.24 2.24 0 0 0-.12.74H5.06a3.82 3.82 0 0 1 .19-1.35 2.11 2.11 0 0 1 .63-.86 4.17 4.17 0 0 0 .66-.67 1.09 1.09 0 0 0 .23-.67.73.73 0 0 0-.77-.86.71.71 0 0 0-.57.26 1.1 1.1 0 0 0-.23.7h-2A2.36 2.36 0 0 1 4 2.47a2.94 2.94 0 0 1 2-.65 3.06 3.06 0 0 1 2 .6 2.12 2.12 0 0 1 .72 1.72 2 2 0 0 1-.17.86z"/></svg>

mozilla / activity-stream Archived

Fix Bug 1422079 - sections context menu #3987

Fix Bug 1422079 - sections context menu #3987

Conversation

rlr commented Feb 19, 2018

rlr commented Feb 20, 2018 • edited

Mardak commented Feb 20, 2018

aaronrbenson commented Feb 20, 2018

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

rlr Feb 21, 2018 Author Contributor

This comment has been minimized.

rlr Feb 21, 2018 Author Contributor

Mardak left a comment

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

csadilek Feb 21, 2018 Collaborator

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

flodolo Feb 21, 2018 Collaborator

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

flodolo Feb 21, 2018 Collaborator

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

rlr Feb 21, 2018 Author Contributor

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

csadilek Feb 21, 2018 Collaborator

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

rlr Feb 21, 2018 Author Contributor

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

rlr Feb 21, 2018 Author Contributor

This comment has been minimized.

flodolo Feb 21, 2018 Collaborator

This comment has been minimized.

flodolo Feb 21, 2018 Collaborator

This comment has been minimized.

flodolo Feb 21, 2018 Collaborator

This comment has been minimized.

Mardak Feb 21, 2018 Member

This comment has been minimized.

rlr Feb 21, 2018 Author Contributor

This comment has been minimized.

flodolo Feb 21, 2018 Collaborator

This comment has been minimized.

csadilek Feb 21, 2018 Collaborator

csadilek left a comment • edited

rlr commented Feb 22, 2018

Mardak left a comment

This comment has been minimized.

Mardak Feb 22, 2018 Member

This comment has been minimized.

Mardak Feb 22, 2018 Member

This comment has been minimized.

csadilek Feb 22, 2018 Collaborator

This comment has been minimized.

csadilek Feb 22, 2018 Collaborator

rlr commented Feb 20, 2018 •

edited

Mardak Feb 21, 2018
Member

rlr Feb 21, 2018
Author Contributor

rlr Feb 21, 2018
Author Contributor

Mardak Feb 21, 2018
Member

Mardak Feb 21, 2018
Member

Mardak Feb 21, 2018
Member

csadilek Feb 21, 2018
Collaborator

Mardak Feb 21, 2018
Member

Mardak Feb 21, 2018
Member

flodolo Feb 21, 2018
Collaborator

Mardak Feb 21, 2018
Member

flodolo Feb 21, 2018
Collaborator

Mardak Feb 21, 2018
Member

Mardak Feb 21, 2018
Member

Mardak Feb 21, 2018
Member

rlr Feb 21, 2018
Author Contributor

Mardak Feb 21, 2018
Member

Mardak Feb 21, 2018
Member

csadilek Feb 21, 2018
Collaborator

Mardak Feb 21, 2018
Member

rlr Feb 21, 2018
Author Contributor

Mardak Feb 21, 2018
Member

rlr Feb 21, 2018
Author Contributor

flodolo Feb 21, 2018
Collaborator

flodolo Feb 21, 2018
Collaborator

flodolo Feb 21, 2018
Collaborator

Mardak Feb 21, 2018
Member

rlr Feb 21, 2018
Author Contributor

flodolo Feb 21, 2018
Collaborator

csadilek Feb 21, 2018
Collaborator

csadilek left a comment •

edited

Mardak Feb 22, 2018
Member

Mardak Feb 22, 2018
Member

csadilek Feb 22, 2018
Collaborator

csadilek Feb 22, 2018
Collaborator

rlr Feb 22, 2018
Author Contributor

Mardak Feb 22, 2018
Member

rlr Feb 22, 2018
Author Contributor

Mardak Feb 22, 2018
Member

rlr Feb 22, 2018
Author Contributor

Mardak Feb 22, 2018
Member

Mardak Feb 22, 2018
Member

sarracini Feb 22, 2018
Contributor

rlr Feb 22, 2018
Author Contributor

rlr Feb 22, 2018
Author Contributor

rlr Feb 22, 2018
Author Contributor

rlr Feb 22, 2018
Author Contributor

Mardak Feb 22, 2018
Member

rlr Feb 22, 2018
Author Contributor

rlr Feb 22, 2018
Author Contributor

Mardak Feb 22, 2018
Member

rlr Feb 22, 2018
Author Contributor

Mardak Feb 22, 2018
Member

Mardak Feb 22, 2018
Member

Mardak Feb 22, 2018
Member

Mardak Feb 22, 2018
Member

Mardak Feb 23, 2018
Member

Mardak Feb 23, 2018
Member

Mardak Feb 23, 2018
Member

rlr Feb 23, 2018
Author Contributor

Mardak Feb 23, 2018
Member

Mardak Feb 23, 2018
Member

Mardak Feb 23, 2018
Member