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

BREAKING (Sidebar): remove duration prop #3336

Merged
merged 2 commits into from
Dec 17, 2018
Merged

BREAKING (Sidebar): remove duration prop #3336

merged 2 commits into from
Dec 17, 2018

Conversation

Fabianopb
Copy link
Member

duration prop has been removed from the Sidebar component. However I've maintained a static variable in the Sidebar class to still provide the default duration of 500 ms that is used to manage the callback cycle when showing and hiding the sidebar.

@layershifter, as animationDuration is now a static prop it can actually be assigned externally (like I've done in the modified tests for this PR). If that's ok, should we add that to the type definitions? It could be done as easily as modifying the Sidebar type declaration in the end of Sidebar.d.ts to something like:

declare const Sidebar: SidebarComponent & { animationDuration: number }

Cheers!
(Closes #3328)

@codecov-io
Copy link

Codecov Report

Merging #3336 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3336   +/-   ##
=======================================
  Coverage   99.89%   99.89%           
=======================================
  Files         170      170           
  Lines        2806     2806           
=======================================
  Hits         2803     2803           
  Misses          3        3
Impacted Files Coverage Δ
src/modules/Sidebar/Sidebar.js 100% <100%> (ø) ⬆️

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 26c660e...a846d90. Read the comment docs.

@Fabianopb
Copy link
Member Author

Friendly ping @layershifter and @levithomason, I'd be happy to get some feedback whenever you guys get some time! :)

Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fabianopb sorry, I'm a little busy now. I don't want to expose this in typings now, we can do this if we will have a request for it.

LGTM, thanks 👍

@layershifter layershifter changed the title Sidebar: remove duration prop BREAKING (Sidebar): remove duration prop Dec 17, 2018
@layershifter layershifter merged commit 6d7feef into Semantic-Org:master Dec 17, 2018
@Fabianopb
Copy link
Member Author

No problem @layershifter, thanks for the fast response! :)

@levithomason
Copy link
Member

Released in semantic-ui-react@0.85.0.

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

Successfully merging this pull request may close these issues.

None yet

4 participants