Material-ui: [Tabs] ink bar does not respect value

[killroy42]

It appears that the ink bar moves on tab click independently of the active tab. If I have a controlled component, for example

It appears the ink bar is not connected to the tab state, but simply listens to mouse events directly.

[lucasbento]

@killroy42: can you please follow the issue template and provide a link to a minimally working environment so we can help you out?

[killroy42]

Apologies, @lucasbento, here is a minimal case to illustrate the (possible) issue:
The tab does not change (correctly, due to static value property) but the inkbar follows clicks nonetheless.

material-ui: v0.16.1
react: v5.3.2
browser: v54.0.2840.99 m

import React, {Component, PropTypes} from 'react';
import {render} from 'react-dom';
import injectTapEventPlugin from 'react-tap-event-plugin'; injectTapEventPlugin();

import {Tabs, Tab} from 'material-ui';
import getMuiTheme from 'material-ui/styles/getMuiTheme';
import baseTheme from 'material-ui/styles/baseThemes/lightBaseTheme';

class App extends Component {
    static childContextTypes = {muiTheme: PropTypes.object}
    getChildContext = () => ({muiTheme: getMuiTheme(baseTheme)})
    render() {
        return (
            <Tabs value={1}>
                <Tab value={0} label="Tab A">Tab A</Tab>
                <Tab value={1} label="Tab B">Tab B</Tab>
            </Tabs>
        );
    }
}

render(<App/>, document.body);

[lucasbento]

@killroy42: that's a really odd issue.

Just tried this here (with onChange prop on <Tabs /> and using state) http://www.webpackbin.com/VJDz8cxZz and it appears to be working, does this solve the problem that you are having?

[killroy42]

In my actual code, I use <Tabs value={raceId} onChange={this.handleRaceSelect}> my change handler update the url, which in turn update the state. But I check for unsaved data and sometimes prevent the tab change. But the ink bar still moves, just as if onChange wasn't set, even though it is. I briefly looked over the source and could not find any connection from the ink bar to the value state management. It seemed as if the inkbar was solely hooked into the tab event handler.

[killroy42]

Ok, I think I'm narrowing it down a bit. My page unload handler prevents a re-render. If you set onChange, but do not call setState in there and cause no render, the tabs will not update, but the inkbar will change. So I am guessing there is something in the tab render that stops the inkbar animation that is triggered by the click event. Perhaps it is possible to trigger the inkbar from the actual tab change, rather than directly off of the mouse event?

In the meantime, adding this.setState({}); to my handler does the trick as a work-around. It doesn't feel quite right, thought ;)

[killroy42]

In fact, this.forceRender(); also does the trick.

[lucasbento]

My page unload handler prevents a re-render.

Page unload?
If you are using shouldComponentUpdate it's better to put a condition there to update if the tab property on state changes.

[killroy42]

Yes, it's a large, complex editing component that handles undo/save operations when the user attempts to navigate away.
The point is, the Tabs inkbar does not honor the tabs value property. This appears to be a bug.

[lucasbento]

I agree, I'll keep this issue opened until we solve that problem.

[killroy42]

I am not sure about patching it myself, but I'm pretty sure that Tabs.handleTabTouchTap(value, event, tab) updates pre-emptively, ignoring the current value property.
It executes this.setState({selectedIndex: index}); unconditionally, which triggers the inkbar update in render() at left={${width * this.state.selectedIndex}%}, while selectedIndex get's reset to the correct value inside getSelectedIndex on the next componentWillReceiveProps, which is why calling forceRender() stops the inkbar, and preventing a re-render moves the inkbar.

... or something like that. Might try a patch after all...

[lucasbento]

@killroy42: sounds good, would love to review your PR. 😁

[oliviertassinari]

An initial migration of the Tabs component has been merged on the next branch.
It's controlled by design, that type of error shouldn't happen now. I'm closing this issue.
If you find a workaround for the master branch. We would still accept a PR for it 👍 .
Thanks.

[oliviertassinari]

In particular, when a Link is followed to a path associated with a tab, the correct tab component is rendered, but the inkbar is not updated.

Is the text changing color? I mean, do you notify the Tabs component that the index changed?

[oliviertassinari]

The name makes me think that it's the default behaviour. I think that it can be confirmed with the documentation or the source code.

[palfvin]

@oliviertassinari I deleted my comments because I realized I completely misread the documentation. We were not setting value at all. Once we did, things worked fine. Sorry for taking up your time. :-)