feat(master-detail): added master detail demo #2645

mattnolting · 2020-01-30T19:19:13Z

https://patternfly-next-pr-2645.surge.sh/documentation/core/experimental/drawer
https://patternfly-next-pr-2645.surge.sh/documentation/core/demos/masterdetail
https://patternfly-next-pr-2645.surge.sh/documentation/core/demos/drawer

patternfly-build · 2020-01-30T19:29:09Z

PatternFly-Next preview: https://patternfly-next-pr-2645.surge.sh

mcoker · 2020-02-05T00:17:56Z

src/patternfly/demos/MasterDetail/master-detail-toolbar.hbs

+  {{/data-toolbar-content}}
+{{/data-toolbar}}
+
+{{!--


can you remove this if it isn't needed?

mcoker · 2020-02-05T00:18:56Z

src/patternfly/sass-utilities/mixins.scss

@@ -192,3 +192,65 @@
    #{$value}: #{$prop};
  }
 }
+
+@mixin pf-label-list-builder($namespace, $row-gap: "sm", $column-gap: "sm", $grid-gap: "md") {


Can you pull this out of this PR and put it in a separate issue or whatever you think is appropriate since we're just exploring the idea currently?

mcoker · 2020-02-06T18:16:38Z

src/patternfly/demos/MasterDetail/examples/MasterDetail.md

+
+## Demos
+
+### isFullscreen


did you mean to add isFullscreen to the end of the ```hbs lines?

mcoker · 2020-02-06T21:10:53Z

src/patternfly/components/Drawer/drawer.scss

+.pf-c-drawer__content-body,
+.pf-c-drawer__panel-body {
+  .pf-c-drawer__action {
+    float: right;


This could create a layout like this where text wraps around the button. like this

Typically whatever is to the left of the button stays to the left of the button, like the modal or popover here

what do you think @mcarrano @mceledonia?

I updated this. The developer will be responsible for placing the close button as some instances control the panel outside of the panel, possibly in the __content or in page header.

mcoker · 2020-02-06T21:14:02Z

src/patternfly/components/Drawer/drawer-action.hbs

+  {{#if drawer-action--attribute}}
+    {{{drawer-action--attribute}}}
+  {{/if}}>
+  {{#> button button--modifier="pf-m-plain" button--id=(concat drawer--id '-close-button') button--attribute=(concat 'id="'  drawer--id '-close-button" aria-labelledby="' drawer--id '-close-button ' drawer--id '-label" aria-label="Close drawer panel"')}}


should we only use aria-labelledby if the -label element exists? That element may not always exist, since the -label isn't an element of the drawer component. (like it isn't in any of these examples https://patternfly-next-pr-2645.surge.sh/documentation/core/beta/drawer)

mcoker · 2020-02-06T21:59:00Z

src/patternfly/components/Drawer/drawer.scss

-      .pf-c-drawer__panel.pf-m-width-#{$width-value}#{$breakpoint-name} {
-        --pf-c-drawer__panel--FlexBasis: #{percentage($width-value / 100)};
+    // Drawer inline
+    .pf-c-drawer.pf-m-inline#{$breakpoint-name} {


I wonder, if the CSS is the same between pf-m-inline and pf-m-static, if we could either include pf-m-static in this loop as a selector if the breakpoint is "base", or maybe make a mixin for the common styles.

mcoker · 2020-02-06T22:04:30Z

src/patternfly/components/Page/page.scss

-    display: flex;
-    flex-direction: column;
-  }
+  // // stylelint-disable max-nesting-depth, selector-max-class, selector-max-compound-selectors


do we need to keep the commented code?

mcoker · 2020-02-06T22:28:55Z

src/patternfly/demos/Drawer/drawer-demo-default.hbs

-      {{/drawer-panel}}
-    {{/drawer}}
-  {{/page-drawer}}
+  {{#if drawer-demo-default--NoPageWrapper}}


Is there a benefit to doing this with and without the page element? If not I think we should pick one way.

mcoker · 2020-02-06T22:47:51Z

src/patternfly/demos/MasterDetail/master-detail-card-data-list.hbs

@@ -0,0 +1,107 @@
+{{#> data-list data-list--id=(concat master-detail-template--id 'data-list') data-list--attribute='aria-label="Selectable rows data list example"'}}


Suggested change

{{#> data-list data-list--id=(concat master-detail-template--id 'data-list') data-list--attribute='aria-label="Selectable rows data list example"'}}

{{#> data-list data-list--id=(concat master-detail-template--id '-data-list') data-list--attribute='aria-label="Selectable rows data list example"'}}

mcoker · 2020-02-06T22:50:38Z

src/patternfly/demos/MasterDetail/examples/MasterDetail.css

+body .ws-example {
+  max-width: none;
+}
+


do you need any of this anymore? Looks like it was needed when the examples weren't full screen

mcoker · 2020-02-06T23:43:05Z

src/patternfly/demos/MasterDetail/examples/MasterDetail.md

+{{/master-detail-template}}
+```
+
+To initiate independently scrollable content and panel sections simply omit `.pf-c-page__section` or other height managed parent element.


This is neat. I wonder if this should be an enhancement to the page section though - that might be useful in other contexts, too. like pf-m-scrollable that sets overflow: auto on the page section. You could define a header or some content in the first page section, then the next page section scrolls, versus the whole content area scrolling. wdyt?

mcoker · 2020-02-06T23:46:06Z

src/patternfly/components/Drawer/drawer-header.hbs

@@ -0,0 +1,6 @@
+<div class="pf-c-drawer__header{{#if drawer-header--modifier}} {{drawer-header--modifier}}{{/if}}"


What is this element used for?

It's part of the master/detail layout concept. This example uses it.

https://marvelapp.com/9iefh5b/screen/63567619

Ah, does that need to be a part of the drawer component though? It looks kinda random. I would expect the space between the text and paragraph in that demo to come from something like a stack layout or a content component that wraps the header text and paragraph and the header text is some sort of heading element.

Doesn't exist anymore. drawer__body goes directly in drawer__content and drawer__panel and replaces drawer__header.

mcarrano

These look mostly great @mattnolting . There were just a couple of things that seemed off.

In the first example (using a table in the master panel), the detail item, Node 2, should be shown selected in the table.
In the Simple List example, the list does not seem to display with correct styling.

I'm also not completely clear on whether this is doing the right thing in mobile state. How do I navigate between the master and detail?

@lboehling can you also take a look here and see if the demos are consistent with your design intent?

mcoker · 2020-02-07T17:06:33Z

@mcarrano @mattnolting the simple list (and notification drawer) styles should be fixed on master now via this PR - #2684. You'll just need to rebase.

mattnolting · 2020-02-07T19:43:02Z

These look mostly great @mattnolting . There were just a couple of things that seemed off.

Thanks!

In the first example (using a table in the master panel), the detail item, Node 2, should be shown selected in the table.

We don't have a selectable table row yet.

In the Simple List example, the list does not seem to display with correct styling.

Updated

I'm also not completely clear on whether this is doing the right thing in mobile state. How do I navigate between the master and detail?

pf-c-drawer__panel is always secondary to pf-c-drawer__content and always contains the dynamic, supporting content. If a table row is selected in __content, the __panel displays the supporting info. On the mobile view, their is only one layout. That is __panel is hidden by default, an element is selected (like a data-list row) then the __panel expands from the right and overlaps the __content. Past the medium breakpoint, __panel will respond to any of the provided modifiers.

@lboehling can you also take a look here and see if the demos are consistent with your design intent?

christiemolloy · 2020-02-07T20:01:33Z

list is getting cut off

mattnolting · 2020-02-07T21:16:36Z

list is getting cut off

That's the default padding for simple list

mcoker · 2020-02-07T21:31:46Z

src/patternfly/components/Drawer/drawer.scss

@@ -1,26 +1,36 @@
 $pf-c-drawer--breakpoint-map: build-breakpoint-map("base", "lg", "xl", "2xl");
-$pf-c-drawer__panel--width-list: (25, 33, 50, 66, 75);
+$pf-c-drawer-width--breakpoint-map: build-breakpoint-map("base", "lg", "xl", "2xl");


that might be better as $pf-c-drawer--breakpoint-map--width (with "width" as a modifier/variation of "breakpoint-map")

mcoker · 2020-02-07T22:24:06Z

src/patternfly/components/Drawer/drawer.scss

+
+  // Action
+  --pf-c-drawer__action--MarginRight: var(--pf-global--spacer--md);
+  --pf-c-drawer__panel-body--c-drawer__action--MarginRight: calc(var(--pf-global--spacer--md) * -1);


I wonder if we should even offer this button if we aren't going to position it for them? I wonder if we should do something like the card - when it has an action menu you use .pf-c-card__head with .pf-c-card__main and .pf-c-card__action, and you put whatever you want to show up on the left in __main. And if you have nothing to put beside the close button, maybe you just leave it blank like this example

I expect a close button to be part of the React implementation, so I'll leave for the demo and remove for the component.

mcoker · 2020-02-07T23:14:02Z

src/patternfly/components/Page/page.scss

@@ -422,6 +422,10 @@
  }
 }

+.pf-c-page__sidebar + .pf-c-drawer {


Were we going to look at using .pf-c-page__drawer here? Seems like we would still want to maintain the other page styles that are in the .pf-c-page__main, .pf-c-page__drawer selector, and for the ability to use page sections and things like that.

mcoker · 2020-02-07T23:23:44Z

src/patternfly/components/Drawer/drawer.scss

+
+// Toolbar in drawer
+.pf-c-drawer .pf-c-data-toolbar {
+  --pf-c-data-toolbar__content--PaddingRight: var(--pf-c-drawer__panel-body--PaddingRight);


Why doesn't the data-toolbar have left/right padding that match the page gutter system (lg on desktop, md on mobile)? I would expect it to. This is what it looks like currently in a demo with the page component and a table.

Also that would allow someone to use the toolbar somewhere else in the drawer in another context without getting these styles inadvertently, since the current styling will apply to all toolbars in a drawer.

Why doesn't the data-toolbar have left/right padding that match the page gutter system (lg on desktop, md on mobile)?

.pf-c-page left/right padding doesn't match most elements within which pf-c-toolbar can/will be used (.pf-c-card, .pf-c-tab-content, .pf-c-wizard, etc) or that it applies to (.pf-c-data-list, .pf-c-table, etc). If that's the expected behavior, then all components with built in padding need to follow that pattern.

mcoker · 2020-02-07T23:25:10Z

src/patternfly/components/Drawer/drawer.scss

+
+  .pf-c-simple-list__item-link {
+    --pf-c-simple-list__item-link--PaddingRight: var(--pf-c-drawer--c-simple-list__item-link--PaddingRight);
+    --pf-c-simple-list__item-link--PaddingLeft: var(--pf-c-drawer--c-simple-list__item-link--PaddingLeft);


I think with the title and link padding, we can make the same change as I mentioned below re: the toolbar's default left/right padding. Have the spacing adjust with the page gutter system so it's 24px on desktop and 16px on mobile. And that could go in the simple list, so we shouldn't need to update it here.

mcoker · 2020-02-07T23:28:44Z

src/patternfly/components/Drawer/drawer.scss

  height: 100%;
  overflow-x: hidden;
+
+  .pf-c-simple-list__title {
+    --pf-c-simple-list__title--PaddingTop: var(--pf-c-drawer--c-simple-list__title--PaddingTop);


I wonder if we can build this in to the simple list itself or maybe create some sort of drawer__panel-nav element or something like that, that is used to create space for some sort of list like this inside of it, similar to this in the sidebar that spaces the nav component from the top.

Then we don't have to re-style the simple list in the drawer, since I presume most drawers don't have a simple list, and it creates a dependency. Also that could add padding to the bottom so we don't end up with this lack of space at the end of the list that looks off

Also that would allow someone to use the simple list somewhere else in the drawer in another context without getting these styles inadvertently, since the current styling will apply to all simple lists in a drawer.

mcarrano · 2020-02-10T16:46:25Z

@mattnolting Thanks for your reply on the issues I raised. Regarding the master-detail with table example, I'm wondering if we should just pull that out if we don't have the ability to select a row in a table. It does not seem very useful in that case. @LHinson are you aware that we have a priority need for this? Is there currently a plan to add selectable rows to the Table component?

mattnolting · 2020-02-11T17:29:14Z

@mcarrano @LHinson demos are using data-list now

mattnolting · 2020-02-11T21:07:13Z

@matthewcarleton big ask, but can you review this behemoth?

mcoker · 2020-02-12T04:09:24Z

src/patternfly/components/Drawer/drawer.scss

-    --pf-c-drawer__panel-body--PaddingLeft: var(--pf-c-drawer__panel-body--md--PaddingLeft);
+  @media screen and (min-width: $pf-global--breakpoint--xl) {
+    --pf-c-drawer__panel--FlexBasis: var(--pf-c-drawer__panel--xl--FlexBasis);
+    --pf-c-drawer__panel--MinWidth: var(--pf-c-drawer__panel--xl--MinWidth);


Should this var be defined outside of this media query? It's used in a declaration but undefined below this breakpoint.

mcoker · 2020-02-12T04:18:37Z

src/patternfly/components/Drawer/drawer.scss

-// Panel body
-.pf-c-drawer__panel-body {
-  padding: var(--pf-c-drawer__panel-body--PaddingTop) var(--pf-c-drawer__panel-body--PaddingRight) var(--pf-c-drawer__panel-body--PaddingBottom) var(--pf-c-drawer__panel-body--PaddingLeft);
+// Modified content children


Just an observation, but this is creating some pretty hefty selectors.

.pf-c-drawer__head, .pf-c-drawer__header, .pf-c-drawer__body { padding: var(--pf-c-drawer--child--PaddingTop) var(--pf-c-drawer--child--PaddingRight) var(--pf-c-drawer--child--PaddingBottom) var(--pf-c-drawer--child--PaddingLeft); } .pf-c-drawer__panel .pf-c-drawer__head:not(.pf-m-no-padding)+.pf-c-drawer__head, .pf-c-drawer__panel .pf-c-drawer__header:not(.pf-m-no-padding)+.pf-c-drawer__head, .pf-c-drawer__panel .pf-c-drawer__body:not(.pf-m-no-padding)+.pf-c-drawer__head, .pf-c-drawer__panel .pf-c-drawer__head:not(.pf-m-no-padding)+.pf-c-drawer__header, .pf-c-drawer__panel .pf-c-drawer__header:not(.pf-m-no-padding)+.pf-c-drawer__header, .pf-c-drawer__panel .pf-c-drawer__body:not(.pf-m-no-padding)+.pf-c-drawer__header, .pf-c-drawer__panel .pf-c-drawer__head:not(.pf-m-no-padding)+.pf-c-drawer__body, .pf-c-drawer__panel .pf-c-drawer__header:not(.pf-m-no-padding)+.pf-c-drawer__body, .pf-c-drawer__panel .pf-c-drawer__body:not(.pf-m-no-padding)+.pf-c-drawer__body { --pf-c-drawer--child--PaddingTop: 0; } .pf-c-drawer__content .pf-c-drawer__head.pf-m-padding+.pf-c-drawer__head.pf-m-padding, .pf-c-drawer__content .pf-c-drawer__header.pf-m-padding+.pf-c-drawer__head.pf-m-padding, .pf-c-drawer__content .pf-c-drawer__body.pf-m-padding+.pf-c-drawer__head.pf-m-padding, .pf-c-drawer__content .pf-c-drawer__head.pf-m-padding+.pf-c-drawer__header.pf-m-padding, .pf-c-drawer__content .pf-c-drawer__header.pf-m-padding+.pf-c-drawer__header.pf-m-padding, .pf-c-drawer__content .pf-c-drawer__body.pf-m-padding+.pf-c-drawer__header.pf-m-padding, .pf-c-drawer__content .pf-c-drawer__head.pf-m-padding+.pf-c-drawer__body.pf-m-padding, .pf-c-drawer__content .pf-c-drawer__header.pf-m-padding+.pf-c-drawer__body.pf-m-padding, .pf-c-drawer__content .pf-c-drawer__body.pf-m-padding+.pf-c-drawer__body.pf-m-padding { --pf-c-drawer--child--PaddingTop: 0; } .pf-c-drawer__content .pf-c-drawer__head, .pf-c-drawer__content .pf-c-drawer__header, .pf-c-drawer__content .pf-c-drawer__body { --pf-c-drawer--child--PaddingTop: 0; --pf-c-drawer--child--PaddingRight: 0; --pf-c-drawer--child--PaddingBottom: 0; --pf-c-drawer--child--PaddingLeft: 0; } .pf-c-drawer__head>.pf-c-drawer__head, .pf-c-drawer__header>.pf-c-drawer__head, .pf-c-drawer__body>.pf-c-drawer__head, .pf-c-drawer__head>.pf-c-drawer__header, .pf-c-drawer__header>.pf-c-drawer__header, .pf-c-drawer__body>.pf-c-drawer__header, .pf-c-drawer__head>.pf-c-drawer__body, .pf-c-drawer__header>.pf-c-drawer__body, .pf-c-drawer__body>.pf-c-drawer__body { --pf-c-drawer--child--PaddingTop: 0; --pf-c-drawer--child--PaddingRight: 0; --pf-c-drawer--child--PaddingBottom: 0; --pf-c-drawer--child--PaddingLeft: 0; } .pf-c-drawer__head.pf-m-no-padding, .pf-c-drawer__header.pf-m-no-padding, .pf-c-drawer__body.pf-m-no-padding { --pf-c-drawer--child--PaddingTop: 0; --pf-c-drawer--child--PaddingRight: 0; --pf-c-drawer--child--PaddingBottom: 0; --pf-c-drawer--child--PaddingLeft: 0; } .pf-c-drawer__head.pf-m-padding, .pf-c-drawer__header.pf-m-padding, .pf-c-drawer__body.pf-m-padding { --pf-c-drawer--child--PaddingTop: var(--pf-c-drawer--child--m-padding--PaddingTop); --pf-c-drawer--child--PaddingRight: var(--pf-c-drawer--child--m-padding--PaddingRight); --pf-c-drawer--child--PaddingBottom: var(--pf-c-drawer--child--m-padding--PaddingBottom); --pf-c-drawer--child--PaddingLeft: var(--pf-c-drawer--child--m-padding--PaddingLeft); }

Updated to:

.pf-c-drawer__header, .pf-c-drawer__body { padding: var(--pf-c-drawer--child--PaddingTop) var(--pf-c-drawer--child--PaddingRight) var(--pf-c-drawer--child--PaddingBottom) var(--pf-c-drawer--child--PaddingLeft); } .pf-c-drawer__header.pf-m-no-padding, .pf-c-drawer__body.pf-m-no-padding { --pf-c-drawer--child--PaddingTop: 0; --pf-c-drawer--child--PaddingRight: 0; --pf-c-drawer--child--PaddingBottom: 0; --pf-c-drawer--child--PaddingLeft: 0; } .pf-c-drawer__header.pf-m-padding, .pf-c-drawer__body.pf-m-padding { --pf-c-drawer--child--PaddingTop: var(--pf-c-drawer--child--m-padding--PaddingTop); --pf-c-drawer--child--PaddingRight: var(--pf-c-drawer--child--m-padding--PaddingRight); --pf-c-drawer--child--PaddingBottom: var(--pf-c-drawer--child--m-padding--PaddingBottom); --pf-c-drawer--child--PaddingLeft: var(--pf-c-drawer--child--m-padding--PaddingLeft); } .pf-c-drawer__header:not(.pf-m-no-padding) + *, .pf-c-drawer__body:not(.pf-m-no-padding) + * { --pf-c-drawer--child--PaddingTop: 0; }

mcoker · 2020-02-12T04:24:51Z

src/patternfly/components/Drawer/examples/Drawer.md

+      <b>Drawer content padding.</b>&nbsp;Lorem ipsum dolor sit amet, consectetur adipiscing elit. Phasellus pretium est a porttitor vehicula. Quisque vel commodo urna. Morbi mattis rutrum ante, id vehicula ex accumsan ut. Morbi viverra, eros vel porttitor facilisis, eros purus aliquet erat, nec lobortis felis elit pulvinar sem. Vivamus vulputate, risus eget commodo eleifend, eros nibh porta quam, vitae lacinia leo libero at magna. Maecenas aliquam sagittis orci, et posuere nisi ultrices sit amet. Aliquam ex odio, malesuada sed posuere quis, pellentesque at mauris. Phasellus venenatis massa ex, eget pulvinar libero auctor pretium. Aliquam erat volutpat. Duis euismod justo in quam ullamcorper, in commodo massa vulputate.
+    {{/drawer-content}}
+    {{> drawer-example-panel}}
+  {{/drawer-main}}
 {{/drawer}}
 ```

 ```hbs title=Modified-panel-padding


I don't see any panel modifiers used in this example

mcoker · 2020-02-12T04:31:29Z

src/patternfly/components/Drawer/drawer.scss

+// Panel head
+.pf-c-drawer__head {
+  display: flex;
+  align-items: center;


What about creating a __head-main element that is used in __head to contain any content adjacent to the __actions element - similar to what we just added to card?

I'm doing that with display: grid

mcoker · 2020-02-12T04:39:05Z

src/patternfly/demos/MasterDetail/master-detail-panel-header.hbs

@@ -0,0 +1,15 @@
+{{#> drawer-head}}
+  {{#> drawer-actions}}


should you wrap this in {{#unless drawer--IsStatic}}{{/unless}} so it isn't in static drawers?

No, the responsive layout needs requires a close button.

christiemolloy · 2020-02-12T14:29:52Z

src/patternfly/components/Drawer/drawer-section.hbs

@@ -0,0 +1,6 @@
+<div class="pf-c-drawer__section{{#if drawer-section--modifier}} {{drawer-section--modifier}}{{/if}}"


@mattnolting I still see this here, are you intending to keep it

christiemolloy · 2020-02-12T14:30:40Z

src/patternfly/components/Drawer/examples/Drawer.md

+      Lorem ipsum dolor sit amet, consectetur adipiscing elit. Phasellus pretium est a porttitor vehicula. Quisque vel commodo urna. Morbi mattis rutrum ante, id vehicula ex accumsan ut. Morbi viverra, eros vel porttitor facilisis, eros purus aliquet erat, nec lobortis felis elit pulvinar sem. Vivamus vulputate, risus eget commodo eleifend, eros nibh porta quam, vitae lacinia leo libero at magna. Maecenas aliquam sagittis orci, et posuere nisi ultrices sit amet. Aliquam ex odio, malesuada sed posuere quis, pellentesque at mauris. Phasellus venenatis massa ex, eget pulvinar libero auctor pretium. Aliquam erat volutpat. Duis euismod justo in quam ullamcorper, in commodo massa vulputate.
+    {{/drawer-content}}
+    {{> drawer-example-panel}}
+  {{/drawer-main}}
 {{/drawer}}
 ```

 ```hbs title=Panel-on-left


Change to: closed-panel-left-example

christiemolloy · 2020-02-12T14:46:13Z

src/patternfly/components/Drawer/drawer-head.hbs

@@ -0,0 +1,6 @@
+<div class="pf-c-drawer__head{{#if drawer-head--modifier}} {{drawer-head--modifier}}{{/if}}"


Now that the actions are vertically centered with the text, the text isn't aligning with the content on the left

christiemolloy · 2020-02-12T14:51:34Z

src/patternfly/components/Drawer/examples/Drawer.md

+      Lorem ipsum dolor sit amet, consectetur adipiscing elit. Phasellus pretium est a porttitor vehicula. Quisque vel commodo urna. Morbi mattis rutrum ante, id vehicula ex accumsan ut. Morbi viverra, eros vel porttitor facilisis, eros purus aliquet erat, nec lobortis felis elit pulvinar sem. Vivamus vulputate, risus eget commodo eleifend, eros nibh porta quam, vitae lacinia leo libero at magna. Maecenas aliquam sagittis orci, et posuere nisi ultrices sit amet. Aliquam ex odio, malesuada sed posuere quis, pellentesque at mauris. Phasellus venenatis massa ex, eget pulvinar libero auctor pretium. Aliquam erat volutpat. Duis euismod justo in quam ullamcorper, in commodo massa vulputate.
+    {{/drawer-content}}
+    {{> drawer-example-panel}}
+  {{/drawer-main}}
 {{/drawer}}
 ```

 ```hbs title=Modified-panel-width


I don't see any modified width here

christiemolloy · 2020-02-12T15:00:50Z

src/patternfly/demos/MasterDetail/examples/MasterDetail.md

+    {{#> drawer
+      drawer--id=(concat master-detail-template--id '-drawer')
+      drawer-panel--IsOpen="true"
+      drawer--modifier="pf-m-inline-on-2xl"


Ok does this relate to this? When inline, you would click an item in the list and then the content would appear on top?

christiemolloy · 2020-02-12T15:01:29Z

src/patternfly/demos/MasterDetail/examples/MasterDetail.md

+    {{#> drawer
+      drawer--id=(concat master-detail-template--id '-drawer')
+      drawer-panel--IsOpen="true"
+      drawer--modifier="pf-m-inline-on-2xl"


I wonder if you should show an example of this at that breakpoint where the content is expanded on top of the list items?

christiemolloy · 2020-02-12T18:54:53Z

Should the close button be in the markup here?

mattnolting · 2020-02-12T19:01:14Z

Should the close button be in the markup here?

Yes, it's necessary for the mobile view, although React may opt to remove. I doubt they will as it would be another thing to manage w/JS.

christiemolloy

PERFECT

christiemolloy · 2020-02-12T19:22:26Z

src/patternfly/components/Drawer/drawer-section.hbs

@@ -0,0 +1,6 @@
+<div class="pf-c-drawer__section{{#if drawer-section--modifier}} {{drawer-section--modifier}}{{/if}}"


AH got it!! Okay thank you so much for explaining that!!!

lboehling

This looks really great @mattnolting!!! Only one thing... In the "master detail collapsed" version, I don't think there needs to be a selected row. Collapsing the drawer would deselect the active item and reselecting an item would open the drawer again. @mcarrano do you think we need this version of the demo?

this looks awesome!

mcarrano · 2020-02-12T20:09:41Z

That's a good question @lboehling . Normally I would say no, however next steps would be to create a React demo that shows the interaction, so it that case would React need this screen to represent the return state after the panel is closed? @mattnolting @christiemolloy I'll defer to you on this one.

mattnolting · 2020-02-12T20:13:38Z

This looks really great @mattnolting!!! Only one thing... In the "master detail collapsed" version, I don't think there needs to be a selected row. Collapsing the drawer would deselect the active item and reselecting an item would open the drawer again. @mcarrano do you think we need this version of the demo?

@lboehling Nice catch! I'll update

mattnolting · 2020-02-13T14:27:54Z

Capturing remaining issues here: #2708

matthewcarleton

LGTM! Just a couple questions :) Nice work on a huge effort @mattnolting

matthewcarleton · 2020-02-13T17:52:07Z

src/patternfly/components/Drawer/drawer.scss

  transition-duration: var(--pf-c-drawer__panel--TransitionDuration);
  transition-property: var(--pf-c-drawer__panel--TransitionProperty);
  -webkit-overflow-scrolling: touch;
+
+  &::after {


What is the purpose of this? It doesn't seem to do anything when removed

It's the border container. When .pf-m-border is applied to parent, the background of this element is painted.

matthewcarleton · 2020-02-13T17:54:21Z

src/patternfly/demos/Drawer/drawer-demo-content.hbs

+      {{#> page-main}}
+        {{#> page-template-breadcrumb}}
+        {{/page-template-breadcrumb}}
+        <a id="main-content-{{page--id}}"></a>


should we include href="#" here?

matthewcarleton

🔥

mcarrano

This all looks great @mattnolting !

One small note for React implementation, I understand that toolbars cannot me made to respond correctly here. But this is something that should be corrected in the React demo.

lboehling

Looks fantastic! :)

redallen · 2020-02-13T19:32:41Z

🎉 This PR is included in version 2.64.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

mattnolting requested review from mcoker and christiemolloy and removed request for mcoker January 30, 2020 21:48

mcoker reviewed Feb 5, 2020

View reviewed changes

mcoker reviewed Feb 6, 2020

View reviewed changes

mattnolting mentioned this pull request Feb 6, 2020

Drawer: updates to support master/detail layout patternfly/patternfly-react#3609

Closed

mcoker reviewed Feb 6, 2020

View reviewed changes

mcarrano requested changes Feb 7, 2020

View reviewed changes

mcoker reviewed Feb 7, 2020

View reviewed changes

feat(master-detail): added generic wrappers to house close button

91c2640

mattnolting requested a review from matthewcarleton February 11, 2020 21:06

feat(master-detail): updated docs

71ea5fc

mcoker reviewed Feb 12, 2020

View reviewed changes

christiemolloy requested changes Feb 12, 2020

View reviewed changes

feat(master-detail): updated per comments

24ac0e0

christiemolloy approved these changes Feb 12, 2020

View reviewed changes

lboehling reviewed Feb 12, 2020

View reviewed changes

feat(master-detail): updated data list selected @lboehling

9b792af

matthewcarleton reviewed Feb 13, 2020

View reviewed changes

feat(master-detail): @matthewcarleton added href

549417e

matthewcarleton approved these changes Feb 13, 2020

View reviewed changes

feat(master-detail): @matthewcarleton removed superfluous link

f7ca13b

mcarrano approved these changes Feb 13, 2020

View reviewed changes

lboehling approved these changes Feb 13, 2020

View reviewed changes

christiemolloy merged commit db30305 into patternfly:master Feb 13, 2020

redallen added the released label Feb 13, 2020

mcoker mentioned this pull request Jan 7, 2021

Static drawer disappears at smaller viewports #3780

Closed

		@@ -0,0 +1,107 @@
		{{#> data-list data-list--id=(concat master-detail-template--id 'data-list') data-list--attribute='aria-label="Selectable rows data list example"'}}

		@@ -0,0 +1,6 @@
		<div class="pf-c-drawer__header{{#if drawer-header--modifier}} {{drawer-header--modifier}}{{/if}}"

		@@ -0,0 +1,6 @@
		<div class="pf-c-drawer__section{{#if drawer-section--modifier}} {{drawer-section--modifier}}{{/if}}"

		@@ -0,0 +1,6 @@
		<div class="pf-c-drawer__head{{#if drawer-head--modifier}} {{drawer-head--modifier}}{{/if}}"

feat(master-detail): added master detail demo #2645

feat(master-detail): added master detail demo #2645

Conversation

mattnolting commented Jan 30, 2020 • edited

patternfly-build commented Jan 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcarrano left a comment

Choose a reason for hiding this comment

mcoker commented Feb 7, 2020

mattnolting commented Feb 7, 2020

christiemolloy commented Feb 7, 2020

mattnolting commented Feb 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcarrano commented Feb 10, 2020

mattnolting commented Feb 11, 2020

mattnolting commented Feb 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattnolting Feb 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christiemolloy commented Feb 12, 2020

mattnolting commented Feb 12, 2020 • edited

christiemolloy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lboehling left a comment

Choose a reason for hiding this comment

mcarrano commented Feb 12, 2020

mattnolting commented Feb 12, 2020

mattnolting commented Feb 13, 2020

matthewcarleton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewcarleton left a comment

Choose a reason for hiding this comment

mcarrano left a comment

Choose a reason for hiding this comment

lboehling left a comment

Choose a reason for hiding this comment

redallen commented Feb 13, 2020

mattnolting commented Jan 30, 2020 •

edited

mattnolting Feb 12, 2020 •

edited

mattnolting commented Feb 12, 2020 •

edited