Skip to content
This repository has been archived by the owner on Jun 28, 2021. It is now read-only.

Somehow header tabs are not working correctly #66

Closed
janpieterz opened this issue Oct 7, 2015 · 10 comments
Closed

Somehow header tabs are not working correctly #66

janpieterz opened this issue Oct 7, 2015 · 10 comments

Comments

@janpieterz
Copy link
Contributor

This might not be an error perse with react-mdl, but I'm unsure where else it would lie as off now.

I've got a couple of components, one is a header and the other one is a specific page.
The header component renders the MDL header tabs components. These work well and seem to be initialized ok (since if you don't they look different).

Then the page content has the tab panels in them.

What happens is that it actually does work, but it does not de-activate the other tabs. So effectively if you switch tabs, it only shows new tabs and keeps the other tabs content also rendered (the .is-active class stays on them).

I'm using React-Router and besides that there's not a lot of things that I think might have any relevance.
If I copy and paste the 'default' example made by mdl it works fine.

Any thoughts?

EDIT:
An image showing it happened in real life. After pressing all tabs this is shown. The content per tab is literally Tab x, and obviously in the image Tab 1- Tab 5 should not be visible there.

image

It does however 'enable' the tabs, so on first visit withouth pressing any tab button it looks like:
image

@faergeek
Copy link
Contributor

faergeek commented Oct 7, 2015

Could you provide reduced test case for that?

@tleunen
Copy link
Owner

tleunen commented Oct 7, 2015

Probably something related with your code with React-router, but if you can share some code, we might be able to help you :)

@janpieterz
Copy link
Contributor Author

I'm working on one as we speak!

@janpieterz
Copy link
Contributor Author

There we go, example repo: https://github.com/janpieterz/Mdl-header-tabs

And I think by creating it all over again I've almost answered my own question, but I'm not a 100% sure. In my official app I use the browser history instead of a hashhistory, but since I serve it off the file system (and browser don't allow histories to be replaced), I use hashhistory, shouldn't make a big difference for this, certainly because the same behavior occurs.

When I have the Tabs right away on the home page, it works fine.
When I have the tabs on the second 'content' page it doesn't work.

My current line of thinking is that those Tabs should only be 'made' there when the tab-panels are there, which makes sense.

But since in the source of the header tabs there is no component upgrade call towards MDL since the whole layout upgrade seems to happen in layout.js and not the individual component like HeaderTabs.

Obviously I'm not going to use the tabs like this in the 'real' application. They'll be added dynamically per page. I had that before as well, but that didn't work either so I reverted to a more hardcoded one, which might bring this problem.

Any thoughts on dynamically adding tabs in the header? It seems like as soon as I load the page directly on the items component it works fine, but as soon as I come from another part it isn't (which is what would happen if those headers are dynamically added, since the layout is already upgraded). Calling the upgrade again in the console doesn't seem to solve it.

I know this is mostly my 'mess' but I'm curious how to tackle this one, any help would be massively appreciated.

@janpieterz
Copy link
Contributor Author

I've been trying, in the example, to get the tabs to be dynamically populated, only way I can see that quickly happening is adding a Flux store like Alt. In case this is handy, anyone an idea to do it simpler?

EDIT: I'm doing that in the real app since that one is all setup for this.

@janpieterz
Copy link
Contributor Author

Alright, I think I've found ít, although I'm not sure about a decent solution:
In the real world example, I currently dynamically add a couple of tabs when navigated to a new page. These don't get upgraded (very easy to see).
I then manually delete the data-upgraded from the mdl-layout div. Then running componentHandler.upgradeAllRegistered(); will work.
Obviously this is not the way to go forward, but I'm also not sure how to solve this.
Since it's all related to the layout, somehow I need to re-initiate the layout's upgrade, but running it without manually deleting that tab didn't work. componentHandler.downgradeElements(document.getElementById("layout")); doesn't seem to do a lot, but perhaps I'm adding the elements to this the wrong way.

@janpieterz
Copy link
Contributor Author

I have it 'working' now, by using a relatively dirty hack provided here:
google/material-design-lite#1165

For future reference:
I edited the material js and exposed the MaterialLayoutTab (since the released version doesn't have it yet), then added in my componentDidUpdate the following code:

var tabs = document.querySelectorAll('.mdl-layout__tab');
    var panels = document.querySelectorAll('.mdl-layout__tab-panel');
    var layout = document.querySelector('.mdl-js-layout');
    var tabbar = document.querySelector('.mdl-layout__tab-bar');    
    for (var i = 0; i < tabs.length; i++) {
      new MaterialLayoutTab(tabs[i], tabs, panels, layout.MaterialLayout);
    }

The HeaderTabs of react-mdl needs to be in the DOM already for this to work.
It certainly works like this, but I have the feeling this logic is more at it's place in a 'lower' library.
If anyone( @tleunen ) has an idea how to build this in react-mdl I'm more than happy to take it on in a pull-request! I browsed around a little bit but didn't find too much dynamic stuff in here so I'd be curious how to handle it. This should, obviously, only take place when mdl's fix is in their release.

I'll leave it open so we can discuss the above possibility, if not feasible/possible/wanted it can be closed!

Thanks for the little push that send me down the drain of issues @ MDL and gave me the solution ;)

More references I found about this:
google/material-design-lite#1665

google/material-design-lite#1340

@faergeek
Copy link
Contributor

faergeek commented Oct 8, 2015

I'm not sure about real issue, but first issue that I see is that tabs' href in / route point to elements that do not exist (and console reports Uncaught TypeError: Cannot read property 'classList' of null) and in /items route they do exist.

@janpieterz
Copy link
Contributor Author

Yeah, that's what happened when I just insert them hard in the header. Second page it does work, but not correctly, although the fix above makes the dynamic ones working, there's just a manual extra step needed to initialize the tabs if the layout is not rendered again.

Jan-Pieter Zoutewelle

On 8 okt. 2015, at 17:49, Sergey Slipchenko notifications@github.com wrote:

I'm not sure about real issue, but first issue that I see is that tabs' href in / route point to elements that do not exist (and console reports Uncaught TypeError: Cannot read property 'classList' of null) and in /items route they do exist.


Reply to this email directly or view it on GitHub.

@tleunen
Copy link
Owner

tleunen commented Oct 9, 2015

Several things here:

  • The implementation of the tabs inside the header is not complete. I wanted to reuse the tabs component instead of using hash to show a content, but I did not continue that way and of course I don't remember why...
  • The reason why the tabs are not working at first, is because it tries to display the container scroll-tab-2 (for example), but it's not in the DOM yet when you're on the homepage. You have to click on the "Example content" link to display the tabs content.
  • Also, the reason why the content stays visible is because the tabs get upgraded without their panels. Because the panels are dynamically added after, the mdl component doesn't know them. To resolve this, you could try to re-upgrade the view but it's really not optimal. That's why I'm not a big fan of the current implementation of the tabs in the header.

I'll put the Tabs in header at the top of my todo :)

@tleunen tleunen changed the title Somehow tabs are not working correctly Somehow header tabs are not working correctly Oct 17, 2015
@tleunen tleunen closed this as completed in 591601f Nov 8, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants