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

Leave animation moves up too high #149

Open
drjosephliu opened this issue May 6, 2017 · 3 comments
Open

Leave animation moves up too high #149

drjosephliu opened this issue May 6, 2017 · 3 comments
Labels

Comments

@drjosephliu
Copy link

Loving the plugin so far, but have an issue with the leave animation. It pushes the child component (in this case, a element) up and to the left. How do I get it to fade in it's current position?

App: https://twitch-react-drhectapus.herokuapp.com/

import React, { Component } from 'react';
import { connect } from 'react-redux';
import FlipMove from 'react-flip-move';
import { selectUser, fetchUser, removeUser } from '../actions/index';

class UsersList extends Component {
  constructor(props) {
    super(props);
    this.state = {
      show: 'all',
    };
    this.fetchInitialUsers(this.props.initialUsers);
  }

  fetchInitialUsers(users) {
    users.map(this.props.fetchUser);
  }

  renderUser(user) {
    const { channelData, streamData } = user;
    return (
      <tr
        key={channelData.display_name}
        onClick={() => this.props.selectUser(user)}
        className='list-item'>
        <td>
          <img src={channelData.logo} className='user-logo' />
        </td>
        <td>
          {channelData.display_name}
        </td>
        <td>
          {streamData.stream ?
            <span className='online'>Online</span> :
            <span className='offline'>Offline</span>}

        </td>
        <span
          className="glyphicon glyphicon-remove"
          onClick={() => this.props.removeUser(user)}></span>
      </tr>
    )
  }

  showOnline() {
    this.setState({
      show: 'online'
    });
  }

  showOffline() {
    this.setState({
      show: 'offline'
    });
  }

  showAll() {
    this.setState({
      show: 'all'
    });
  }

  render() {
    return (
      <div className='col-sm-4'>
        <div className='text-center'>
          <div className='btn-group btn-group-sm' role='group'>
            <button
              className='btn btn-default'
              onClick={this.showAll.bind(this)}>
              All
            </button>
            <button
              className='btn btn-default'
              onClick={this.showOnline.bind(this)}>
              Online
            </button>
            <button
              className='btn btn-default'
              onClick={this.showOffline.bind(this)}>
              Offline
            </button>
          </div>
        </div>
        <div className='container'>
        <table className='table table-hover'>
          <thead>
            <tr>
              <th>Logo</th>
              <th>Channel</th>
              <th>Status</th>
            </tr>
          </thead>
          {/* <tbody> */}
            <FlipMove
              typeName='tbody' enterAnimation='fade'
              leaveAnimation='fade'>
              {this.props.users.filter(user => {
                const { show } = this.state;
                const { streamData } = user;
                if (show == 'online') {
                  return streamData.stream;
                }
                else if (show == 'offline') {
                  return !streamData.stream;
                }
                else {
                  return user;
                }
              }).map(this.renderUser.bind(this))}
            </FlipMove>
          {/* </tbody> */}
        </table>
      </div>
      </div>
    )
  }
}

function mapStateToProps({ users, initialUsers }) {
  return { users, initialUsers };
}

export default connect(mapStateToProps, { selectUser, fetchUser, removeUser })(UsersList);

@joshwcomeau
Copy link
Owner

Hey there,

Ok, so I poked around on your web app (cool project, by the way!). The problem appears to be with <tbody> tags not respecting CSS positions.

Short version

FlipMove doesn't work great with HTML tables. The recommended approach would be to use <div>s instead of tables. I recognize that this is a lame suggestion, though, since tables are the most semantic markup for what you're trying to do, but tables really don't like to have their children positioned about.

I have a workaround that might work and allow you to use tables, though. It involves getting rid of the <thead> and <tbody> tags, since they're the problematic ones:

  <FlipMove
    typeName='table' 
    className='table table-hover'
    enterAnimation='fade'
    leaveAnimation='fade'
  >
    <tr>
      <th>Logo</th>
      <th>Channel</th>
      <th>Status</th>
    </tr>

    {this.props.users.filter(user => {
      const { show } = this.state;
      const { streamData } = user;
      if (show == 'online') {
        return streamData.stream;
      }
      else if (show == 'offline') {
        return !streamData.stream;
      }
      else {
        return user;
      }
    }).map(this.renderUser.bind(this))}
  </FlipMove>

This loses the benefit of distinct separation between <thead> and <tbody>, though.

More info

So, leave animations in FlipMove are kinda tricky. Because the sibling elements need to move into the vacuum created by their exit, FlipMove has to remove them from the DOM flow, so that the siblings can figure out where to go.

I do this by setting position: absolute, to take it out of the flow calculations, and then using top and left to place it exactly where it was. This is typically an invisible change to the user, since the node doesn't actually move, it just goes from sitting in the DOM flow to floating above it, in the same position.

position: absolute works by setting offsets from its closest parent that isn't position: static (the default). FlipMove gets around this by applying position: relative to the wrapping element, in this case the <tbody>.

Unfortunately it looks like browsers don't respect the position of a <tbody> tag, because the browser really doesn't expect you to be artificially positioning table rows. So the glitch you're seeing is the rows being positioned relative to the closest ancestor with a set position: the col-sm-4 a couple levels above.

The fix I proposed above should work because the <table> supports position tags, so we just have to set it to relative. The <thead> gets in the way, though, so we have to remove that. That way, FlipMove's calculations don't have anything in the way to correctly set their position before exiting.

I think it would be nice to throw a console.warning if a user tries using FlipMove with a tbody, to prevent others from hitting the same issue and not understanding why it doesn't work. Thanks for bringing this to my attention.

Going to leave this issue open until I've added such a warning. Let me know if the solution without the <tbody> works!

@joshwcomeau joshwcomeau added the bug label May 7, 2017
@drjosephliu
Copy link
Author

Hey @joshwcomeau

I implemented your fix by removing the tbody and thead tags and it's working a lot better in that it's exiting at the same height level now. But the leave animation is still fading out to the left slightly.

App: https://twitch-react-drhectapus.herokuapp.com/

@joshwcomeau
Copy link
Owner

Hey @drhectapus,

So yeah, it looks like this is simply a limit with HTML tables :( When you apply position: absolute to a <tr>, it "undoes" all the table logic of spacing out the cells evenly.

It makes sense, in a way; tr works by aligning with other trs in the same table, and when you set something to position: absolute, you take it out of that flow, so all the cells collapse.

I think this is one of those situations where you'll need to choose between the flexibility of an HTML table, and animations. If you convert the structure to <div>s, FlipMove would work, but you'd have to handle aligning all the columns yourself :(

Leaving this open until I add a note to the docs and a dev warning.

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

No branches or pull requests

2 participants