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(Dimmer): add transition #2001

Closed
wants to merge 6 commits into from
Closed

Conversation

layershifter
Copy link
Member

WIP

@levithomason
Copy link
Member

Very cool, can't wait to get Transitions in the entire framework :) This is largely what is stopping a v1 release. The other blocking work is minor in comparison.

@tim-soft
Copy link

I have been scratching my head over the dimmer transitions used in the docs site, can't wait for this!

@levithomason
Copy link
Member

It would be great if we tackled #837 and made transition a prop on every component possible. We'd just need a HOC that did something like (warning, untested!):

import React from 'react'
import Transition from './modules/Transition/Transition'

const withTransition = Component => {
  const WithTransition = props => {
    const { transition } = props

    const element = <Component {...props} />

    if (!transition) return element

    return Transition.create(transition, {
      defaultProps: { 
        ...props,
        children: element,
      },
    })
  }

  return WithTransition
}

This requires adding a shorthand factory to the Transition.

Then, in our component files:

// Button.js
export default withTransition(Button)

Now, we can do this:

<Button transition='fade' />
<Button transition={{ animation: 'fade', duration: 100 }} />

I actually did this for popup and I cannot find the branch 😭 The only issue I had was getting the docs to understand extended propTypes. Example, the Button above would need to spread the withTransition propTypes. I guess we could just hard code them, but it would be nice to be able to inherit. Especially as we'll have propTypes from withPopup and any other HOCs we add.

@layershifter
Copy link
Member Author

In fact, the key problem that I see there is with page Dimmer. Instead of regular in renders inside Portal which unmounts its content when it closes. I'm still have no idea how to resolve this without tons of callbacks.

@layershifter
Copy link
Member Author

Needs #2155.

@levithomason
Copy link
Member

TransitionablePortal has been merged, unblocking this.

@padrefuture
Copy link

New to this site, and Im hoping this is the right place to post this. Im looking for a a menu with an animated (transition?) dropdown, that has a delay on mouseout. ANy ideas where i can find or is this too much to ask in the SemanticUI/React world?

@levithomason
Copy link
Member

@padrefuture animations are a work in progress here in the React port of Semantic UI. That feature is not yet available. We do have a Transition component. That component needs to be integrated into our components so we have the transitions you are looking for. PRs welcome!

@ghost
Copy link

ghost commented Jan 29, 2018

Am I wrong or is just a linting problem the latest ci/circleci problem?

Can we force this and then fix the lint problems?

I'm newbie, sorry for maybe wrong questions.

@codecov-io
Copy link

codecov-io commented Feb 14, 2018

Codecov Report

Merging #2001 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2001      +/-   ##
=========================================
+ Coverage   99.74%   99.8%   +0.06%     
=========================================
  Files         160     149      -11     
  Lines        2751    2595     -156     
=========================================
- Hits         2744    2590     -154     
+ Misses          7       5       -2
Impacted Files Coverage Δ
src/modules/Dimmer/Dimmer.js 100% <100%> (ø) ⬆️
src/modules/Transition/Transition.js 100% <100%> (ø) ⬆️
src/modules/Dimmer/DimmerContent.js 100% <100%> (ø)
src/views/Comment/CommentGroup.js 100% <0%> (ø) ⬆️
src/elements/Reveal/Reveal.js 100% <0%> (ø) ⬆️
src/elements/Header/HeaderContent.js 100% <0%> (ø) ⬆️
src/elements/Reveal/RevealContent.js 100% <0%> (ø) ⬆️
src/elements/Image/Image.js 100% <0%> (ø) ⬆️
src/modules/Dropdown/DropdownMenu.js 100% <0%> (ø) ⬆️
src/elements/List/ListList.js 100% <0%> (ø) ⬆️
... and 89 more

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 36221c0...b325cc6. Read the comment docs.

@levithomason
Copy link
Member

I've merged master and fixed the lint issues. This PR still appears to be in progress, as the description states.

We'll need the feature completed, some tests, and some docs.

@jermsam
Copy link

jermsam commented Apr 10, 2018

Hey guys. Any success with @levithomason 's sugestion? I don't see myself moving away from semantic ui react just because of a simple animation. This css framework is awesome for react. someone do something please

@brianespinosa
Copy link
Member

Good morning @jermsam

This is an open source project and anyone is welcome to contribute. If there is an area of the framework where you have a need for something specific to your project, pull requests are always welcome.

@stale
Copy link

stale bot commented Jul 9, 2018

There has been no activity in this thread for 90 days. While we care about every issue and we’d love to see this fixed, the core team’s time is limited so we have to focus our attention on the issues that are most pressing. Therefore, we will likely not be able to get to this one.

However, PRs for this issue will of course be accepted and welcome!

If there is no more activity in the next 90 days, this issue will be closed automatically for housekeeping. To prevent this, simply leave a reply here. Thanks!

@stale stale bot added the stale label Jul 9, 2018
@ghost
Copy link

ghost commented Jul 9, 2018

@Stale, please, shut up.

@stale stale bot removed the stale label Jul 9, 2018
@stale
Copy link

stale bot commented Jan 6, 2019

There has been no activity in this thread for 180 days. While we care about every issue and we’d love to see this fixed, the core team’s time is limited so we have to focus our attention on the issues that are most pressing. Therefore, we will likely not be able to get to this one.

However, PRs for this issue will of course be accepted and welcome!

If there is no more activity in the next 180 days, this issue will be closed automatically for housekeeping. To prevent this, simply leave a reply here. Thanks!

@stale stale bot added the stale label Jan 6, 2019
@stale
Copy link

stale bot commented Jul 27, 2019

There has been no activity in this thread for 180 days. While we care about every issue and we’d love to see this fixed, the core team’s time is limited so we have to focus our attention on the issues that are most pressing. Therefore, we will likely not be able to get to this one.

However, PRs for this issue will of course be accepted and welcome!

If there is no more activity in the next 180 days, this issue will be closed automatically for housekeeping. To prevent this, simply leave a reply here. Thanks!

@stale stale bot added the stale label Jul 27, 2019
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.

7 participants