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(Sidebar): add lifecycle handlers #2845

Merged
merged 35 commits into from
Jun 21, 2018

Conversation

layershifter
Copy link
Member

Requires #2837.
Replaces #2433, #1473.


This PR:

  • adds onHide, onHidden, onShow and onVisible callbacks to allow the deep control of visible prop
  • reworks Sidebar docs, examples are much better and clear now 👍
  • improves code coverage

Especially thanks to @cjsheets and @mchirkin for their work in previous PRs!

mchirkin and others added 10 commits May 13, 2018 16:52
…g/Semantic-UI-React into feat/sidebar-closable-property

# Conflicts:
#	src/lib/eventStack/EventPool.js
#	src/lib/eventStack/EventSet.js
#	src/lib/eventStack/EventStack.js
#	src/lib/eventStack/EventTarget.js
#	test/specs/lib/eventStack/EventPool-test.js
#	test/specs/lib/eventStack/EventStack-test.js
…g/Semantic-UI-React into feat/sidebar-closable-property

# Conflicts:
#	src/lib/eventStack/EventPool.js
#	src/lib/eventStack/EventSet.js
#	src/lib/eventStack/EventStack.js
#	src/lib/eventStack/EventTarget.js
#	test/specs/lib/eventStack/EventPool-test.js
#	test/specs/lib/eventStack/EventStack-test.js
@levithomason
Copy link
Member

#2387 merged, unblocked this PR.

@levithomason
Copy link
Member

Heads up, I've reworked the directories in /docs. This PR needs updated. There is a detailed upgrade guide in the PR, #2863.

Oleksandr Fediashov added 3 commits June 4, 2018 10:46
Signed-off-by: Oleksandr Fediashov <[email protected]>
…React into feat/sidebar-closable-property

Signed-off-by: Oleksandr Fediashov <[email protected]>

# Conflicts:
#	docs/app/Examples/modules/Sidebar/Overlay/SidebarExampleRightOverlay.js
#	docs/app/Examples/modules/Sidebar/Push/SidebarExampleRightPush.js
#	docs/app/Examples/modules/Sidebar/ScaleDown/SidebarExampleRightScaleDown.js
#	docs/src/components/ComponentDoc/ComponentControls/ComponentControlsMaximize.js
#	docs/src/components/ComponentDoc/ComponentDocHeader.js
#	docs/src/components/ComponentDoc/ComponentDocLinks.js
#	docs/src/components/ComponentDoc/ComponentDocSee.js
#	docs/src/components/ComponentDoc/ComponentExample/ComponentExampleTitle.js
#	docs/src/components/ComponentDoc/ComponentProp/ComponentPropDefaultValue.js
#	docs/src/components/ComponentDoc/ComponentProp/ComponentPropDescription.js
#	docs/src/components/ComponentDoc/ComponentProp/ComponentPropName.js
#	docs/src/components/ComponentDoc/ComponentProps/ComponentProps.js
#	docs/src/components/ComponentDoc/ComponentProps/ComponentPropsComponent.js
#	docs/src/components/ComponentDoc/ComponentProps/ComponentPropsComponents.js
#	docs/src/components/ComponentDoc/ComponentProps/ComponentPropsDescription.js
#	docs/src/components/ComponentDoc/ComponentProps/ComponentPropsHeader.js
#	docs/src/components/ComponentDoc/ComponentSidebar/ComponentSidebarItem.js
#	docs/src/components/DocsRoot.js
#	docs/src/components/IconSearch/IconSearch.js
#	docs/src/examples/addons/Responsive/Usage/ResponsiveExampleBreakpoints.js
#	docs/src/examples/collections/Form/Shorthand/FormExampleFieldControlId.js
#	docs/src/examples/collections/Form/Shorthand/FormExampleSubcomponentId.js
#	docs/src/examples/collections/Menu/Types/MenuExampleText.js
#	docs/src/examples/collections/Menu/Types/MenuExampleVerticalText.js
#	docs/src/examples/collections/Menu/Variations/MenuExampleAttachedTabular.js
#	docs/src/examples/elements/Button/Groups/ButtonExampleGroupIconShorthand.js
#	docs/src/examples/elements/Button/Types/ButtonExampleBasic.js
#	docs/src/examples/elements/Button/Types/ButtonExampleInverted.js
#	docs/src/examples/elements/Button/Variations/ButtonExampleSize.js
#	docs/src/examples/elements/Container/Types/ContainerExampleContainer.js
#	docs/src/examples/elements/Container/Types/ContainerExampleText.js
#	docs/src/examples/elements/Container/Variations/ContainerExampleAlignment.js
#	docs/src/examples/elements/Container/Variations/ContainerExampleFluid.js
#	docs/src/examples/elements/Header/Variations/HeaderExampleColored.js
#	docs/src/examples/elements/Header/Variations/HeaderExampleInverted.js
#	docs/src/examples/elements/Image/Variations/ImageExampleCentered.js
#	docs/src/examples/elements/Image/Variations/ImageExampleFloated.js
#	docs/src/examples/elements/Image/Variations/ImageExampleSpaced.js
#	docs/src/examples/elements/Label/Types/LabelExampleTag.js
#	docs/src/examples/elements/Label/Variations/LabelExampleCircular.js
#	docs/src/examples/elements/Label/Variations/LabelExampleCircularEmpty.js
#	docs/src/examples/elements/List/Types/ListExampleOrderedValue.js
#	docs/src/examples/elements/Segment/Variations/SegmentExampleAttached.js
#	docs/src/examples/elements/Segment/Variations/SegmentExampleColoredInverted.js
#	docs/src/examples/elements/Segment/Variations/SegmentExampleFloated.js
#	docs/src/examples/elements/Segment/Variations/SegmentExampleTextAlignment.js
#	docs/src/examples/elements/Step/Groups/StepExampleGroupShorthand.js
#	docs/src/examples/modules/Dropdown/States/DropdownExampleActive.js
#	docs/src/examples/modules/Dropdown/States/DropdownExampleDisabled.js
#	docs/src/examples/modules/Dropdown/States/DropdownExampleError.js
#	docs/src/examples/modules/Dropdown/States/DropdownExampleLoading.js
#	docs/src/examples/modules/Dropdown/Usage/DropdownExampleCloseOnChange.js
#	docs/src/examples/modules/Popup/Variations/PopupExampleWide.js
#	docs/src/examples/modules/Sidebar/Examples/SidebarExampleMultiple.js
#	docs/src/examples/modules/Sidebar/Overlay/SidebarExampleBottomOverlay.js
#	docs/src/examples/modules/Sidebar/Overlay/SidebarExampleRightOverlay.js
#	docs/src/examples/modules/Sidebar/Overlay/SidebarExampleTopOverlay.js
#	docs/src/examples/modules/Sidebar/Overlay/index.js
#	docs/src/examples/modules/Sidebar/Push/SidebarExampleBottomPush.js
#	docs/src/examples/modules/Sidebar/Push/SidebarExampleRightPush.js
#	docs/src/examples/modules/Sidebar/Push/SidebarExampleTopPush.js
#	docs/src/examples/modules/Sidebar/Push/index.js
#	docs/src/examples/modules/Sidebar/ScaleDown/SidebarExampleBottomScaleDown.js
#	docs/src/examples/modules/Sidebar/ScaleDown/SidebarExampleRightScaleDown.js
#	docs/src/examples/modules/Sidebar/ScaleDown/SidebarExampleTopScaleDown.js
#	docs/src/examples/modules/Sidebar/ScaleDown/index.js
#	docs/src/examples/modules/Sidebar/SlideAlong/SidebarExampleRightSlideAlong.js
#	docs/src/examples/modules/Sidebar/SlideAlong/index.js
#	docs/src/examples/modules/Sidebar/SlideOut/SidebarExampleRightSlideOut.js
#	docs/src/examples/modules/Sidebar/SlideOut/index.js
#	docs/src/examples/modules/Sidebar/States/SidebarExampleDimmed.js
#	docs/src/examples/modules/Sidebar/Types/SidebarExampleSidebar.js
#	docs/src/examples/modules/Sidebar/Uncover/SidebarExampleRightUncover.js
#	docs/src/examples/modules/Sidebar/Uncover/index.js
#	docs/src/examples/modules/Sidebar/index.js
#	docs/src/examples/views/Feed/Types/FeedExampleEventsProp.js
#	docs/src/examples/views/Statistic/Variations/StatisticExampleFloated.js
#	docs/src/hoc/neverUpdate.js
#	docs/src/hoc/pure.js
#	docs/src/hoc/updateForKeys.js
#	docs/src/hoc/withDocInfo.js
#	docs/src/utils/getComponentGroup.js
#	docs/src/utils/getNewHash.js
#	docs/src/utils/getSeeItems.js
#	docs/src/utils/isOldHash.js
#	docs/src/utils/parentComponents.js
#	src/modules/Sidebar/Sidebar.js
#	test/specs/commonTests/implementsClassNameProps.js
#	test/specs/commonTests/isConformant.js
Signed-off-by: Oleksandr Fediashov <[email protected]>
@layershifter layershifter force-pushed the feat/sidebar-closable-property branch from 62e0ada to 7c3541b Compare June 4, 2018 08:34
Oleksandr Fediashov added 9 commits June 4, 2018 11:35
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Oleksandr Fediashov added 8 commits June 4, 2018 12:03
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
Signed-off-by: Oleksandr Fediashov <[email protected]>
@codecov-io
Copy link

codecov-io commented Jun 5, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@43793ae). Click here to learn what that means.
The diff coverage is 93.54%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2845   +/-   ##
=========================================
  Coverage          ?   99.78%           
=========================================
  Files             ?      161           
  Lines             ?     2760           
  Branches          ?        0           
=========================================
  Hits              ?     2754           
  Misses            ?        6           
  Partials          ?        0
Impacted Files Coverage Δ
src/modules/Sidebar/Sidebar.js 94.59% <93.54%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43793ae...86cf279. Read the comment docs.

@layershifter
Copy link
Member Author

@levithomason marked as ready for review 👍

@levithomason
Copy link
Member

In http://localhost:8080/modules/sidebar#types-sidebar, clicking outside does not close the Sidebar. It seems onHide is called twice, resulting in a double toggle.

If I create a handleHide method that sets visible to false only and assign that to onHide, then it works. However, we should ensure the onHide is not called twice.

@layershifter
Copy link
Member Author

Will check, sounds stange 🤔

@layershifter
Copy link
Member Author

I made some updates, now it works as expected.

@levithomason levithomason merged commit 0d00166 into master Jun 21, 2018
@levithomason levithomason deleted the feat/sidebar-closable-property branch June 21, 2018 04:32
@levithomason
Copy link
Member

Released in [email protected].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants