Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(drawer): Added the drawer component to experimental #2633

Merged
merged 20 commits into from Aug 9, 2019

Conversation

@dlabaj
Copy link
Contributor

dlabaj commented Aug 2, 2019

Fixes #1938

Added drawer component to experimental features similar to what was done for core (https://pf4.patternfly.org/components/Drawer/examples/)

TODO:

Adding unit and component tests now will update this PR with those tests.

@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Aug 2, 2019

PatternFly-React preview: https://patternfly-react-pr-2633.surge.sh

Copy link
Contributor

redallen left a comment

Can't vouch for styles, but LGTM.

Copy link
Member

mcarrano left a comment

This looks good @dlabaj , but I'm a bit confused about what's different about the two examples. The first one should overlay content on the page and the second would push page content into a smaller container, right? I can't really tell from these examples if that's what's happening since it looks like the page content and the drawer are not competing for the same space.

@dlabaj

This comment has been minimized.

Copy link
Contributor Author

dlabaj commented Aug 2, 2019

This looks good @dlabaj , but I'm a bit confused about what's different about the two examples. The first one should overlay content on the page and the second would push page content into a smaller container, right? I can't really tell from these examples if that's what's happening since it looks like the page content and the drawer are not competing for the same space.

@mcarrano The styles are messed up due to the import of pattternfly.css and base.css coming before the drawer component. Trying to figure out how to fix it now.

dlabaj added 3 commits Aug 5, 2019
@dlabaj dlabaj removed the Do Not Merge label Aug 5, 2019
@dlabaj

This comment has been minimized.

Copy link
Contributor Author

dlabaj commented Aug 5, 2019

@mcarrano It's fixed now can you take another look at the docs.

</Alert>
<br />

## Simple Drawer Component

This comment has been minimized.

Copy link
@tlabaj

tlabaj Aug 5, 2019

Contributor

Sentence case her please

This comment has been minimized.

Copy link
@dlabaj

dlabaj Aug 5, 2019

Author Contributor

Done.

packages/patternfly-4/react-docs/gatsby-browser.js Outdated Show resolved Hide resolved
@tlabaj tlabaj added the Do Not Merge label Aug 5, 2019
dlabaj added 2 commits Aug 5, 2019
@dlabaj dlabaj removed the Do Not Merge label Aug 5, 2019
@mcarrano

This comment has been minimized.

Copy link
Member

mcarrano commented Aug 5, 2019

Updates look good @dlabaj . This may be more of a CSS issue, but ion the second (inline) example, should there be some padding between the edge of the drawer and the content area? Thoughts @mcoker ?

@mcoker

This comment has been minimized.

Copy link
Contributor

mcoker commented Aug 5, 2019

This may be more of a CSS issue, but ion the second (inline) example, should there be some padding between the edge of the drawer and the content area?

@mcarrano that's a good point. On second thought, seems like both the main element (holds the main content) and drawer element (hold's the drawer panel's content) should have padding by default, and the option to remove it, if the user wants to add their own layout that has its own padding. What do you think of that?

@@ -15,7 +15,7 @@ export const DrawerPanelContent: React.SFC<DrawerPanelContentProps> = ({
...props
}: DrawerPanelContentProps) => (
<aside className={css(styles.drawerPanel, className)} {...props}>
<div className={css(styles.drawerPanelBody)}>
<div className={css(styles.drawerPanelBody, styles.modifiers.noPadding)}>

This comment has been minimized.

Copy link
@mcoker

mcoker Aug 8, 2019

Contributor

looks like this just adds the modifier? It should be a prop you can define for the panel content that applies this class. Without the prop defined (default), or set to false, the panel should have padding. With the prop defined and not set to false, the panel should have no padding around it (as it does right now).

FWIW, we'll be adding the same type of prop for the drawer content element, too, but I need to make that change in core first.

This comment has been minimized.

Copy link
@dlabaj

dlabaj Aug 9, 2019

Author Contributor

okay will make this update

/** Indicate if the drawer is expanded */
isExpanded: boolean;
/** Indicates if the content element and panel element are displayed side by side. */
isInline: boolean;

This comment has been minimized.

Copy link
@jschuler

jschuler Aug 8, 2019

Collaborator

should be optional?

dlabaj and others added 8 commits Aug 8, 2019
…rawer/DrawerPanelContent.tsx

Co-Authored-By: Joachim <jschuler@redhat.com>
…rawer/Drawer.tsx

Co-Authored-By: Joachim <jschuler@redhat.com>
…rawer/Drawer.tsx

Co-Authored-By: Joachim <jschuler@redhat.com>
Copy link
Contributor

tlabaj left a comment

LGTM

@dlabaj dlabaj dismissed stale reviews from tlabaj and jschuler via 4e9e48e Aug 9, 2019
@mcoker
mcoker approved these changes Aug 9, 2019
Copy link
Contributor

mcoker left a comment

nice!!

@jschuler jschuler merged commit ff6d905 into patternfly:master Aug 9, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Aug 9, 2019

Your changes have been released in:

  • @patternfly/react-core@3.85.0
  • @patternfly/react-docs@4.10.0
  • @patternfly/react-inline-edit-extension@2.9.73
  • demo-app-ts@2.18.0
  • @patternfly/react-integration@2.18.0
  • @patternfly/react-table@2.16.12
  • @patternfly/react-topology@2.7.21
  • @patternfly/react-virtualized-extension@1.1.107

Thanks for your contribution! 🎉

@tlabaj
tlabaj approved these changes Aug 9, 2019
Copy link
Contributor

tlabaj left a comment

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.