Bug 1421225 - Clicking on a console.group Message should toggle the group. r=nchevobbe draft
authorabhinav <abhinav.koppula@gmail.com>
Tue, 23 Jan 2018 09:28:15 +0530
changeset 737853 224bcfa41da64bf7c745da807ef6a396c152e51e
parent 723950 b0baaec09caf3e1b30ec6b238f5c46ef9b3188be
push id96784
push userbmo:abhinav.koppula@gmail.com
push dateThu, 25 Jan 2018 08:43:25 +0000
reviewersnchevobbe
bugs1421225
milestone60.0a1
Bug 1421225 - Clicking on a console.group Message should toggle the group. r=nchevobbe MozReview-Commit-ID: IuK0R1Ebv8o
devtools/client/shared/components/Frame.js
devtools/client/themes/webconsole.css
devtools/client/webconsole/new-console-output/components/Message.js
devtools/client/webconsole/new-console-output/test/components/console-api-call.test.js
devtools/client/webconsole/new-console-output/test/components/page-error.test.js
--- a/devtools/client/shared/components/Frame.js
+++ b/devtools/client/shared/components/Frame.js
@@ -214,16 +214,17 @@ class Frame extends Component {
     }, sourceElements);
 
     // If source is not a URL (self-hosted, eval, etc.), don't make
     // it an anchor link, as we can't link to it.
     if (isLinkable) {
       sourceEl = dom.a({
         onClick: e => {
           e.preventDefault();
+          e.stopPropagation();
           onClick(this.getSourceForClick(frame));
         },
         href: source,
         className: "frame-link-source",
         draggable: false,
       }, sourceInnerEl);
     } else {
       sourceEl = dom.span({
--- a/devtools/client/themes/webconsole.css
+++ b/devtools/client/themes/webconsole.css
@@ -1258,9 +1258,13 @@ body #output-container {
 }
 
 .sidebar-close-button::before {
   background-image: var(--close-button-image);
 }
 
 .sidebar-contents .object-inspector {
   min-width: 100%;
+}
+
+.theme-twisty {
+  cursor: default;
 }
\ No newline at end of file
--- a/devtools/client/webconsole/new-console-output/components/Message.js
+++ b/devtools/client/webconsole/new-console-output/components/Message.js
@@ -71,16 +71,17 @@ class Message extends Component {
     return {
       indent: 0
     };
   }
 
   constructor(props) {
     super(props);
     this.onLearnMoreClick = this.onLearnMoreClick.bind(this);
+    this.toggleMessage = this.toggleMessage.bind(this);
     this.onContextMenu = this.onContextMenu.bind(this);
   }
 
   componentDidMount() {
     if (this.messageNode) {
       if (this.props.scrollToMessage) {
         this.messageNode.scrollIntoView();
       }
@@ -93,44 +94,51 @@ class Message extends Component {
     }
   }
 
   onLearnMoreClick(e) {
     let {exceptionDocURL} = this.props;
     this.props.serviceContainer.openLink(exceptionDocURL, e);
   }
 
+  toggleMessage(e) {
+    let { open, dispatch, messageId } = this.props;
+    if (open) {
+      dispatch(actions.messageClose(messageId));
+    } else {
+      dispatch(actions.messageOpen(messageId));
+    }
+  }
+
   onContextMenu(e) {
     let { serviceContainer, source, request, messageId } = this.props;
     let messageInfo = {
       source,
       request,
       messageId,
     };
     serviceContainer.openContextMenu(e, messageInfo);
     e.stopPropagation();
     e.preventDefault();
   }
 
   render() {
     const {
-      messageId,
       open,
       collapsible,
       collapseTitle,
       source,
       type,
       level,
       indent,
       topLevelClasses,
       messageBody,
       frame,
       stacktrace,
       serviceContainer,
-      dispatch,
       exceptionDocURL,
       timeStamp = Date.now(),
       timestampsVisible,
       notes,
     } = this.props;
 
     topLevelClasses.push("message", source, type, level);
     if (open) {
@@ -165,23 +173,17 @@ class Message extends Component {
     }
 
     // If there is an expandable part, make it collapsible.
     let collapse = null;
     if (collapsible) {
       collapse = CollapseButton({
         open,
         title: collapseTitle,
-        onClick: function () {
-          if (open) {
-            dispatch(actions.messageClose(messageId));
-          } else {
-            dispatch(actions.messageOpen(messageId));
-          }
-        },
+        onClick: this.toggleMessage
       });
     }
 
     let notesNodes;
     if (notes) {
       notesNodes = notes.map(note => dom.span(
         { className: "message-flex-body error-note" },
         dom.span({ className: "message-body devtools-monospace" },
@@ -248,17 +250,20 @@ class Message extends Component {
       },
       "aria-live": type === MESSAGE_TYPE.COMMAND ? "off" : "polite"
     },
       timestampEl,
       MessageIndent({indent}),
       icon,
       collapse,
       dom.span({ className: "message-body-wrapper" },
-        dom.span({ className: "message-flex-body" },
+        dom.span({
+          className: "message-flex-body",
+          onClick: collapsible && this.toggleMessage,
+        },
           // Add whitespaces for formatting when copying to the clipboard.
           timestampEl ? " " : null,
           dom.span({ className: "message-body devtools-monospace" },
             ...bodyElements,
             learnMore
           ),
           repeat ? " " : null,
           repeat,
--- a/devtools/client/webconsole/new-console-output/test/components/console-api-call.test.js
+++ b/devtools/client/webconsole/new-console-output/test/components/console-api-call.test.js
@@ -299,16 +299,70 @@ describe("ConsoleAPICall component:", ()
       ));
       wrapper.find(".theme-twisty").simulate("click");
       call = store.dispatch.getCall(1);
       expect(call.args[0]).toEqual({
         id: message.id,
         type: MESSAGE_OPEN
       });
     });
+
+    it("toggle the group when the group name is clicked", () => {
+      const store = setupStore();
+      store.dispatch = sinon.spy();
+      const message = stubPreparedMessages.get("console.group('bar')");
+
+      let wrapper = mount(Provider({store},
+        ConsoleApiCall({
+          message,
+          open: true,
+          dispatch: store.dispatch,
+          serviceContainer,
+        })
+      ));
+      wrapper.find(".message-flex-body").simulate("click");
+      let call = store.dispatch.getCall(0);
+      expect(call.args[0]).toEqual({
+        id: message.id,
+        type: MESSAGE_CLOSE
+      });
+
+      wrapper = mount(Provider({store},
+        ConsoleApiCall({
+          message,
+          open: false,
+          dispatch: store.dispatch,
+          serviceContainer,
+        })
+      ));
+      wrapper.find(".message-flex-body").simulate("click");
+      call = store.dispatch.getCall(1);
+      expect(call.args[0]).toEqual({
+        id: message.id,
+        type: MESSAGE_OPEN
+      });
+    });
+
+    it("doesn't toggle the group when the location link is clicked", () => {
+      const store = setupStore();
+      store.dispatch = sinon.spy();
+      const message = stubPreparedMessages.get("console.group('bar')");
+
+      let wrapper = mount(Provider({store},
+        ConsoleApiCall({
+          message,
+          open: true,
+          dispatch: store.dispatch,
+          serviceContainer,
+        })
+      ));
+      wrapper.find(".frame-link-source").simulate("click");
+      let call = store.dispatch.getCall(0);
+      expect(call).toNotExist();
+    });
   });
 
   describe("console.groupEnd", () => {
     it("does not show anything", () => {
       const message = stubPreparedMessages.get("console.groupEnd('bar')");
       const wrapper = render(ConsoleApiCall({ message, serviceContainer }));
 
       expect(wrapper.find(".message-body").text()).toBe("");
--- a/devtools/client/webconsole/new-console-output/test/components/page-error.test.js
+++ b/devtools/client/webconsole/new-console-output/test/components/page-error.test.js
@@ -71,17 +71,21 @@ describe("PageError component:", () => {
 
   it("displays a [Learn more] link", () => {
     const store = setupStore();
 
     const message = stubPreparedMessages.get("ReferenceError: asdf is not defined");
 
     serviceContainer.openLink = sinon.spy();
     const wrapper = mount(Provider({store},
-      PageError({message, serviceContainer})
+      PageError({
+        message,
+        serviceContainer,
+        dispatch: () => {},
+      })
     ));
 
     // There should be a [Learn more] link.
     const url =
       "https://developer.mozilla.org/docs/Web/JavaScript/Reference/Errors/Not_defined";
     const learnMore = wrapper.find(".learn-more-link");
     expect(learnMore.length).toBe(1);
     expect(learnMore.prop("title")).toBe(url);