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

Components: Popover, add story #18096

Merged
merged 5 commits into from Nov 19, 2019
Merged

Conversation

@ItsJonQ
Copy link
Contributor

ItsJonQ commented Oct 24, 2019

Components: Popover, add story

Screen Capture on 2019-10-24 at 17-35-22

This update adds a story for the Popover component.
Knobs were used to better demonstrate the properties.

An additional story (positioning) is added to demonstrate the
auto-positioning feature of Popover. This can be seen by dragging
the gray box around from within the story.

This update adds a story for the Popover component.
Knobs were used to better demonstrate the properties.

An additional story (positioning) is added to demonstrate the
auto-positioning feature of Popover. This can be seen by dragging
the gray box around from within the story.
Copy link
Member

jorgefilipecosta left a comment

Hi @ItsJonQ, thank you for expanding our Storybook component coverage. The PR seems good I just left some small suggestions.

return <Popover { ...props } />;
};

const DragExample = ( props ) => {

This comment has been minimized.

Copy link
@jorgefilipecosta

jorgefilipecosta Oct 28, 2019

Member

Would it be possible to expose the Drag&Drop mechanism as something generic other stories can use?

This comment has been minimized.

Copy link
@ItsJonQ

ItsJonQ Oct 28, 2019

Author Contributor

@jorgefilipecosta Thank you so much!!

Would it be possible to expose the Drag&Drop mechanism as something generic other stories can use?

I can certainly create something and export it. In the past, I used this library for the stories:
https://github.com/mzabriskie/react-draggable

For this PR, I wrote out the bare bones functionality manually to keep it simple :).

};

const startDragging = () => setDragging( true );
const stopDragging = () => setDragging( false );

This comment has been minimized.

Copy link
@jorgefilipecosta

jorgefilipecosta Oct 28, 2019

Member

We are generating new functions startDragging, stopDragging, and updatePosition. Could we wrap these functions with useCallback to avoid that?

This comment has been minimized.

Copy link
@ItsJonQ

ItsJonQ Oct 28, 2019

Author Contributor

@jorgefilipecosta Ah yes! Thank you. Will make that update

@ItsJonQ ItsJonQ self-assigned this Oct 28, 2019
@ItsJonQ

This comment has been minimized.

Copy link
Contributor Author

ItsJonQ commented Oct 28, 2019

@jorgefilipecosta I just pushed an update to address your suggestions! (Thanks again)
Let me know if this works :)

Copy link
Member

jorgefilipecosta left a comment

LGTM 👍

@ItsJonQ ItsJonQ merged commit ca0c0eb into WordPress:master Nov 19, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@ItsJonQ ItsJonQ deleted the ItsJonQ:add/popover-stories branch Nov 19, 2019
@youknowriad youknowriad added this to the Gutenberg 7.0 milestone Nov 25, 2019
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.