Material-ui: [Tabs] Tab indicator in Tabs behaves wrong when tabs are dynamically changed

[thupi]

• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

The tab indicator must always show the selected tab

Current Behavior

When the number of tabs in a tab menu changes the indicator have a wrong position until next interaction such as clicking a new tab.

Steps to Reproduce (for bugs)

1. Go to https://codesandbox.io/s/2pm5jzmk10
2. Select a random tab
3. Click switch
4. Chehck indicator position

Context

I Noticed this issue when trying to remove some tabs based on the users permission role. I quickly realised that the indicator was not in sync with the amount of tabs inside the menu.

Reproduction Environment

| Tech | Version |
|--------------|---------|
| Material-UI |1.0.0-beta.11 |
| React |15.5.3 |
| browser | Google Chrome v61 |

Your Environment

| Tech | Version |
|--------------|---------|
| Material-UI |1.0.0-beta.6 |
| React |15.6.1 |
| browser | Google Chrome v61 |

[oliviertassinari]

We might want to change this logic:
https://github.com/callemall/material-ui/blob/438dd7f7fc14f504f0e385fef1719ee6674fa3ca/src/Tabs/Tabs.js#L164-L167

[thupi]

@oliviertassinari Would it be too bad performance to remove the if statement arround :-) ?

I would really like to contribute but is relativly new in the open source world :-) Is there any guides to start contributing in generel :-) ?

[oliviertassinari]

@thupi This issue might not have been the simpler issue to start contributing with. I have been doing some cleanup along the way. Thanks for your interest. Don't miss the CONTRIBUTING.md file and the issues with the good first issue flag 👍

[sriramrudraraju]

Im getting similar issue in production build.
generated project from "create-react-app"
material-ui: beta 16
react: v16

it working as expected on dev build. but when i use production build, indicator highlighting 2nd tab(my case). but once i start clicking the tabs everything works fine.

any suggestions or workarounds. i tried v0.19.4, it worked great in production build too.

Thanks..

[rlyle]

Just to add, I'm using objects as my "value" and the indicator refuses to be under the currently selected tab.

Note in the above image, the current selection is "Allergies", but the indicator is highlighing the "Insights" tab. This is in chrome with material-ui 1.0.0.-beta-.20.

[lucas-viewup]

The problem is only in indicator, the value props is changing as expected, I am having the same problem of the guy above. Here it is my component:

import React from 'react';
import PropTypes from 'prop-types';
import { withStyles } from 'material-ui/styles';
import Tabs, { Tab } from 'material-ui/Tabs';
import Typography from 'material-ui/Typography';

const styles = theme => ({
  root: {
    flexGrow: 1,
    backgroundColor: theme.palette.background.paper,
  },
  tabsRoot: {
    borderBottom: '1px solid #e8e8e8',
  },
  tabsIndicator: {
    backgroundColor: '#f00',
    display: 'none'
  },
  tabRoot: {
    textTransform: 'initial',
    fontWeight: theme.typography.fontWeightRegular,
    marginRight: theme.spacing.unit * 4,
    '&:hover': {
      color: '#40a9ff',
      opacity: 1,
    },
    '&$tabSelected': {
      color: '#1890ff',
      fontWeight: theme.typography.fontWeightMedium,
      background: 'red',
    },
    '&:focus': {
      color: '#40a9ff',
    },
  },
  tabSelected: {},
  typography: {
    padding: theme.spacing.unit * 3,
  },
});


const TabsItems = (props) => {
    let components = [];

    const {classes, items, onChange, ...other} = props;

    items.map( (item, index) => {
        components.push(
            <Tab
                key={index}
                disableRipple
                onChange={onChange}
                classes={{ root: classes.tabRoot, selected: classes.tabSelected }}
                {...item}
            />
        );
    });


    return components;
};

class CustomTabs extends React.Component {
  state = {
    value: 0,
  };

  handleChange = (event, value) => {
    console.log('bang!');
    this.setState({ value });
  };

  render() {
    const { classes, tabs, ...other } = this.props;
    const { value } = this.state;

    return (
        <div className={classes.root} {...other}>
            <Tabs
            value={value}
            active
            onChange={this.handleChange}
            classes={{ root: classes.tabsRoot, indicator: classes.tabsIndicator }}
            >
                <TabsItems classes={classes} onChange={this.handleChange} items={tabs} />                
            </Tabs>
            <Typography className={classes.typography}>Ant Design</Typography>
        </div>
    );
  }
}

CustomTabs.propTypes = {
  classes: PropTypes.object.isRequired,
};

export default withStyles(styles)(CustomTabs);

[oliviertassinari]

@lucas-viewup You can try the action property of the Tabs and pull out an updateIndicator() function.

[ljani]

I had the very same problem as @lucas-viewup and @rlyle. My problem was that I was using functions as values for the tabs and that is not supported although the documentation for Tab and Tabs say that the value can be any type.

The problem is that the backing implementation is using an object to store the values. Thus the values must provide an unique string representation. I see these possible fixes:

• Fix the documentation to say the values will be converted to strings (or even make the API only accept strings) and one should make sure they're unique. We also could probably warn if the value was not unique when it's added to valueToIndex?
• Change the backing store to a Map. Is that acceptable?

In my case, all of the values were converted to the following string and thus they were not unique:

"function bound value() {
    [native code]
}"

This string representation was only produced in the production build and during development time the values were unique.

[ljani]

@oliviertassinari Hey, I know you're busy, but I noticed I didn't highlight you in my previous comment, so I just wanted to make sure you noticed my comment.

[oliviertassinari]

Change the backing store to a Map. Is that acceptable?

@ljani I confirm the issue you are facing. Yes, using a Map would be much better. Do you want to work on it?

[ljani]

@oliviertassinari Great! I'll try and see if I can get something done next week.