Bug 90789

Created attachment 151408 [details] Patch

Comment on attachment 151408 [details] Patch Attachment 151408 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13187037

Created attachment 151412 [details] Patch

Comment on attachment 151412 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151412&action=review > Source/WebCore/ChangeLog:15 > + * inspector/front-end/CSSNamedFlowsSidebarPane.js: Added. This also needs to be added to WebKit.qrc > Source/WebCore/inspector/front-end/CSSNamedFlowsSidebarPane.js:6 > + WebInspector.SidebarPane.call(this, WebInspector.UIString("CSS Named Flows")); Localized strings need to be added to Source/WebCore/English.lproj/localizedStrings.js in the alphabetical order

Created attachment 151432 [details] Patch

Comment on attachment 151432 [details] Patch Could you please give some details on your plans for that feature? Ideally we would like to see a screenshot. Anyway this boilerplate seems plausible but I don't think we should land it in such an early stage.

Created attachment 151447 [details] Workflow The archive contains the workflow for some use cases. Please ignore the label for the elements tab (upper-left corner). It will remain "Elements". There are clickable areas in the web pages for exploring the workflow.

Could you please be more verbose and explain what this feature is about. We need to figure out if it belongs to the code inspector or not (should be a part of the extension).

(In reply to comment #8) s/the code inspector/the core inspector/

This feature aims to: * display an overview of the named flows in a page - each named flow will have a "content" section (elements that flow into) and a "region chain" section (elements that flow from) * make it possible to select a node in the DOM tree displayed in the "Elements" tab by selecting it in the named flow overview - you can expand a named flow in the overview and, by selecting an element found in the "content" or the "region chain" section, it should be selected in the DOM tree * display a visual cue that represents a region's overset - in the "region chain" section, a region with "overset: fit" will be displayed differently from one with "overset: empty" * display a visual cue when a named flow does not fit in its region chain * highlight a region - by hovering over a region, in the "region chain" section, it should be highlighted in the web page Other use cases: * highlight an entire region chain * display the rules applied from region styling when selecting an element that flows inside a region * display the @region styling rules when selecting a box that is also a region * display a subtree of the content that flows into a region

I don't think this information fits the sidebar well. I'd rather use WebInspector.showViewInDrawer() for that. You should implement your own WebInspector.View with the desired information and show it there upon the context menu action (just like "local modifications"). It is critical that we agree upon the looks of this view upfront - we need fairly detailed UI mocks for that. And it would be interesting to learn how you intend to retrieve necessary information from the backend (will you need to expand the protocol for that)? Lets fill this bug with the details on the high-level looks of the view and protocol extension proposal before we go on.

Created attachment 151708 [details] Here are some screenshots with the first features, from the UI point of view. I will also post details about the extension of the protocol.

Screenshot 1: Could we make "CSS named flow" available in the context menu for elements from the flow? I imagine there can be 30 flows in the page and we need to be able to quickly navigate to the one of interest. In your view, you could have a list of the flows to the left and the flow content to the right. Similar to the Network or Sources panel, but in the drawer. Screenshot 2: Names of flows look cryptic. You need to give a hint to the user. Like Flow "first" Flow "second" Also, hovering over the flow should highlight it in the screen. Screenshot 5: Automatic "Reveal in the Elements Panel" context menu action will do that, clicking should have no effect Screenshot 7: Rendering a warning icon (yellow triangle) would be appropriate Otherwise, looks like a descent start.

Created attachment 152525 [details] Screenshots The new design. What do you think about adding a button on the bottom status bar for opening/closing the drawer (like the console) ?

Created attachment 152527 [details] Protocol extension The protocol may require further extension in order to support other use cases. Is there a way to highlight multiple css boxes at the same time?

(In reply to comment #14) > Created an attachment (id=152525) [details] > Screenshots > > The new design. > What do you think about adding a button on the bottom status bar for opening/closing the drawer (like the console) ? I don't think we should add top-level UI elements for narrow features like this. (In reply to comment #15) > Created an attachment (id=152527) [details] > Protocol extension > > The protocol may require further extension in order to support other use cases. > Is there a way to highlight multiple css boxes at the same time? Nope, but we could extend existing protocol to do that.

Created attachment 152749 [details] Mockup of CSS regions in styles pane Thanks for the mocks, I think this is a good start. However, it's pretty undiscoverable to put this in a context menu on the Elements pane, and I'm not sure we need a separate panel for this. I would expect the main entry point for this UI to be from an element that is part of a named flow. What about showing the information inline in the styles pane? I'm also not sure that grey is the right way to communicate an empty region. Grey usually means disabled.

(In reply to comment #17) > Created an attachment (id=152749) [details] > Mockup of CSS regions in styles pane > > Thanks for the mocks, I think this is a good start. However, it's pretty undiscoverable to put this in a context menu on the Elements pane, and I'm not sure we need a separate panel for this. I would expect the main entry point for this UI to be from an element that is part of a named flow. > > What about showing the information inline in the styles pane? > > I'm also not sure that grey is the right way to communicate an empty region. Grey usually means disabled. The problem with showing the information in the styles panel is that you will not have an overview of all the named flows in the page. Also, by revealing another region from the same named flow (using the info in the styles pane), the styles pane is updated and it may be difficult to refocus on that named flow. I think it is a good idea to extend the styles pane in the way you proposed, but we should also find a way to display the information from the drawer.

(In reply to comment #18) > The problem with showing the information in the styles panel is that you will not have an overview of all the named flows in the page. Can you suggest a scenario where that would be really useful? I'm having trouble imagining one. I would think that when debugging, you'd be wondering either "Why isn't this element flowing into this region?" or "Why doesn't this region contain any content?". > Also, by revealing another region from the same named flow (using the info in the styles pane), the styles pane is updated and it may be difficult to refocus on that named flow. That's true. However, I think there are better ways to solve this problem. And, it's not impossible, only inconvenient. > I think it is a good idea to extend the styles pane in the way you proposed, but we should also find a way to display the information from the drawer. My feeling is that we should start with the minimal UI, and then consider making improvements after we get a better sense of how it is used. I think we should try to to add any more special-purpose panes unless it's absolutely necessary. Pavel, what do you think?

> My feeling is that we should start with the minimal UI, and then consider making improvements after we get a better sense of how it is used. I think we should try to to add any more special-purpose panes unless it's absolutely necessary. > > Pavel, what do you think? @Patrick: I am not sure I follow the question. Did you mean "should not"? Are you suggesting sidebar or a drawer?

(In reply to comment #20) > > My feeling is that we should start with the minimal UI, and then consider making improvements after we get a better sense of how it is used. I think we should try to to add any more special-purpose panes unless it's absolutely necessary. > > > > Pavel, what do you think? > > @Patrick: I am not sure I follow the question. Did you mean "should not"? Are you suggesting sidebar or a drawer? Sorry, I meant to say "I think we should try NOT to add any more special-purpose panes." I'd suggesting only using the sidebar for now.

(In reply to comment #20) > > My feeling is that we should start with the minimal UI, and then consider making improvements after we get a better sense of how it is used. I think we should try to to add any more special-purpose panes unless it's absolutely necessary. > > > > Pavel, what do you think? > > @Patrick: I am not sure I follow the question. Did you mean "should not"? Are you suggesting sidebar or a drawer? Sorry, I meant to say "I think we should try NOT to add any more special-purpose panes." I'd suggest only using the sidebar for now.

> Sorry, I meant to say "I think we should try NOT to add any more special-purpose panes." I'd suggest only using the sidebar for now. I think the opposite. We are talking about the functionality that is very targeted (used by the small percentage of the users). Hence it should not be visible / on the screen at all times. - Sidebar is something that is always visible, it exposes functionality one needs on a daily basis. Styles, Metrics, fall here. - Temporary drawer views are sessional, invisible by default and can scale. They are opened using contextual actions. Advanced search results, revision history fall here. We can also expose custom drawers using extensions API. Eventually, we'll have a tabbed pane there and allow several views in drawer at a time (a-la Eclipse's views). So I think drawer is a better fit this time.

(In reply to comment #23) > > Sorry, I meant to say "I think we should try NOT to add any more special-purpose panes." I'd suggest only using the sidebar for now. > > I think the opposite. We are talking about the functionality that is very targeted (used by the small percentage of the users). Hence it should not be visible / on the screen at all times. Right. My proposal was also not visible by default -- you'd have to drill down into a "-webkit-flow-from" or "-webkit-flow-into" style to see the info. > - Sidebar is something that is always visible, it exposes functionality one needs on a daily basis. Styles, Metrics, fall here. Since CSS regions are just styles, it seems logical to expose the information inside the Style pane, as in my mockup. Do you agree, or do you think that it should only be accessible from the context menu? > - Temporary drawer views are sessional, invisible by default and can scale. They are opened using contextual actions. Advanced search results, revision history fall here. We can also expose custom drawers using extensions API. Eventually, we'll have a tabbed pane there and allow several views in drawer at a time (a-la Eclipse's views). The problem with the drawers is that they take up a lot of screen real estate, which is especially bad in compact mode. Ideally, I think they should be mostly transient. If we try to restrict their use to cases where they are absolutely necessary, then hopefully most of the time the drawer won't need to be open. Also, I think that the ability to view the _default_ document flow would be tremendously useful in debugging layouts. That's not an expert feature, so it should be easily discoverable. I think we should use the same UI for exploring both named flows and the default flow, hence my suggestion that we implement minimal support for named flows for now.

(In reply to comment #24) > (In reply to comment #23) > > > Sorry, I meant to say "I think we should try NOT to add any more special-purpose panes." I'd suggest only using the sidebar for now. > > > > I think the opposite. We are talking about the functionality that is very targeted (used by the small percentage of the users). Hence it should not be visible / on the screen at all times. > > Right. My proposal was also not visible by default -- you'd have to drill down into a "-webkit-flow-from" or "-webkit-flow-into" style to see the info. > > > - Sidebar is something that is always visible, it exposes functionality one needs on a daily basis. Styles, Metrics, fall here. > > Since CSS regions are just styles, it seems logical to expose the information inside the Style pane, as in my mockup. Do you agree, or do you think that it should only be accessible from the context menu? Ok, now that I see the mock, what you are saying makes more sense. I would not want to add anything heavy into the narrow sidebar. You screenshot is speculatively wide! > > > - Temporary drawer views are sessional, invisible by default and can scale. They are opened using contextual actions. Advanced search results, revision history fall here. We can also expose custom drawers using extensions API. Eventually, we'll have a tabbed pane there and allow several views in drawer at a time (a-la Eclipse's views). > > The problem with the drawers is that they take up a lot of screen real estate, which is especially bad in compact mode. Ideally, I think they should be mostly transient. If we try to restrict their use to cases where they are absolutely necessary, then hopefully most of the time the drawer won't need to be open. I perceive this functionality as profiling. I.e. you are going to spend time exploring these flows and you need real estate there. Another important point is that putting it into a drawer significantly reduces the integration surface and raises our chances to form this functionality as an extension. > > Also, I think that the ability to view the _default_ document flow would be tremendously useful in debugging layouts. That's not an expert feature, so it should be easily discoverable. I think we should use the same UI for exploring both named flows and the default flow, hence my suggestion that we implement minimal support for named flows for now. Unfortunately, I am not sure what default flow is and how one debugs it. Also not sure what the difference between the named flows and default one is. I'd suggest that the patch contributor explains in greater detail the life of the flow debugging. Like what information user gets and what he/she can change live and how. But from the common sense standpoint, I'd suspect that implementing it in a rectangular view would be more flexible.

(In reply to comment #19) > (In reply to comment #18) > > The problem with showing the information in the styles panel is that you will not have an overview of all the named flows in the page. > > Can you suggest a scenario where that would be really useful? I'm having trouble imagining one. I would think that when debugging, you'd be wondering either "Why isn't this element flowing into this region?" or "Why doesn't this region contain any content?". The overview might come in handy when you are trying to get the big picture on how some web page is designed, especially if it is a complex one. > > Also, by revealing another region from the same named flow (using the info in the styles pane), the styles pane is updated and it may be difficult to refocus on that named flow. > > That's true. However, I think there are better ways to solve this problem. And, it's not impossible, only inconvenient. If a developer really wants to, he can debug a web page that uses css regions without any extension of the web inspector: css regions come down to some css properties. The problem that people who design css regions samples pointed out is that it is still a difficult task. > I'd suggest that the patch contributor explains in greater detail the life of the flow debugging. Like what information user gets and what he/she can change live and how. But from the common sense standpoint, I'd suspect that implementing it in a rectangular view would be more flexible. Useful information when debugging a web page that uses named flows: * the overset property of a named flow (and how this property changes on events like resizing) * the elements that flow into and from a named flow * the overset property of a region * the order in which data from a named flow is placed in the regions * the position of a region in the web page * the position of the region chain in the web page

(In reply to comment #26) > (In reply to comment #19) > > (In reply to comment #18) > > > The problem with showing the information in the styles panel is that you will not have an overview of all the named flows in the page. > > > > Can you suggest a scenario where that would be really useful? I'm having trouble imagining one. I would think that when debugging, you'd be wondering either "Why isn't this element flowing into this region?" or "Why doesn't this region contain any content?". > > The overview might come in handy when you are trying to get the big picture on how some web page is designed, especially if it is a complex one. > > > > Also, by revealing another region from the same named flow (using the info in the styles pane), the styles pane is updated and it may be difficult to refocus on that named flow. > > > > That's true. However, I think there are better ways to solve this problem. And, it's not impossible, only inconvenient. > > If a developer really wants to, he can debug a web page that uses css regions without any extension of the web inspector: css regions come down to some css properties. The problem that people who design css regions samples pointed out is that it is still a difficult task. > > > I'd suggest that the patch contributor explains in greater detail the life of the flow debugging. Like what information user gets and what he/she can change live and how. But from the common sense standpoint, I'd suspect that implementing it in a rectangular view would be more flexible. > > Useful information when debugging a web page that uses named flows: > * the overset property of a named flow (and how this property changes on events like resizing) > * the elements that flow into and from a named flow > * the overset property of a region > * the order in which data from a named flow is placed in the regions > * the position of a region in the web page > * the position of the region chain in the web page Andrei, thanks for the info. After discussing with Pavel offline, I agree that it makes sense to implement this in a drawer.

This was fixed by achicu a while ago in the current inspector.