Navigation Menu

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

tabs onShow function executes when new tab and old tab are active #6467

Open
philipraets opened this issue Oct 17, 2019 · 6 comments
Open

tabs onShow function executes when new tab and old tab are active #6467

philipraets opened this issue Oct 17, 2019 · 6 comments

Comments

@philipraets
Copy link
Contributor

I use a function for the onShow property of the tabs, which will recalculate the heigth of the page.
But onShow gets executed after the new tab-content is set to active and before the old tab-content is set to non-active.
So the height of the page will be higher than expected, because the 2 tabs are active at the moment.

Expected Behavior

Expected is that the onShow function will be executed after the old tab-content is set to non-active.

Current Behavior

Right now it will be executed between the switching of tab-content.

Possible Solution

in tabs.js change:

      // Swap content
      if (this.options.swipeable) {
        if (this._tabsCarousel) {
          this._tabsCarousel.set(this.index, () => {
            if (typeof this.options.onShow === 'function') {
              this.options.onShow.call(this, this.$content[0]);
            }
          });
        }
      } else {
        if (this.$content.length) {
          this.$content[0].style.display = 'block';
          this.$content.addClass('active');
           if (typeof this.options.onShow === 'function') {
            this.options.onShow.call(this, this.$content[0]);
          }

          if ($oldContent.length && !$oldContent.is(this.$content)) {
            $oldContent[0].style.display = 'none';
            $oldContent.removeClass('active');
          }


        }
      }

to

      // Swap content
      if (this.options.swipeable) {
        if (this._tabsCarousel) {
          this._tabsCarousel.set(this.index, () => {
            if (typeof this.options.onShow === 'function') {
              this.options.onShow.call(this, this.$content[0]);
            }
          });
        }
      } else {
        if (this.$content.length) {
          this.$content[0].style.display = 'block';
          this.$content.addClass('active');

          if ($oldContent.length && !$oldContent.is(this.$content)) {
            $oldContent[0].style.display = 'none';
            $oldContent.removeClass('active');
          }

           if (typeof this.options.onShow === 'function') {
            this.options.onShow.call(this, this.$content[0]);
          }

        }
      }

Steps to Reproduce (for bugs)

I had the following js in my page:

function test(){
    console.log(document.body.clientHeight);
}
var tabs = document.getElementById("tabs");
var instance = M.Tabs.init(tabs, {onShow: test});

Context

my tab-content doesn't have a fixed height, and my table of contents bottom has to be recalculated on changing tabs, or it flow into the footer of the page

Your Environment

  • Version used: 1.00
  • Browser Name and version: firefox 69
  • Operating System and version (desktop or mobile): openSUSE Tumbleweed desktop
  • Link to your project (if appropriate):
@pwcreative
Copy link

Ideally you need to provide a Codepen.

How about delaying your function by 500ms via setTimeout?

@philipraets
Copy link
Contributor Author

I've created a codepen which shows what happens:

https://codepen.io/philipraets/pen/yLLgNmQ

I recalculate the height of the document when the tab is shown, so the table of contents won't flow into my footer.

at the moment when the onShow-function gets executed, there are 2 tabs with active content, so the height of the document is too high.

@pwcreative
Copy link

pwcreative commented Oct 21, 2019

Thanks for the codepen but that is way to complex for an example. You need to strip it back to the bare bones and highlight exactly what element/function is causing the problem. I have no idea what is going on in that example.

Have you tried delaying the function that generates the page height by a fraction of a second, so that the calculation is performed when the active states are correct? Pretty sure that will solve your problem.

EDIT: I was correct :)

https://codepen.io/doughballs/pen/yLLgYQB

function test(){
  setTimeout(function(){
    counter++;
  console.log("running " + counter + " times");
  var exampleDiv = document.getElementById("example");
  exampleDiv.style.height = '30px';
  exampleDiv.innerHTML = 'Height: 30px';
  var footerHeight = document.getElementsByTagName("footer")[0].offsetHeight;
  var documentHeight = 0;
  documentHeight = document.body.clientHeight;
  var documentHeightNoFooter = documentHeight-footerHeight;
  console.log(documentHeight);
  console.log(documentHeightNoFooter);
  console.log("test");
  exampleDiv.style.height = documentHeightNoFooter + 'px';
  exampleDiv.innerHTML = 'Height: ' + documentHeightNoFooter + 'px';
  console.log("einde test");
  tableOfContents();
  },500)
}

@philipraets
Copy link
Contributor Author

I've cleaned up the codepen and added comments to the javascript.
The time-out is working, although this just feels like a workaround and shouldn't be needed.

I've created the test-function in the codepen to show the result, but in reality, the tableOfContents function will be called on onShow.

I thought that onShow would be fired when the tab is active and the other tab is hidden, not when they are both active.

Thanks for looking into it

@pwcreative
Copy link

pwcreative commented Oct 21, 2019

It is definitely a workaround but to get a component to behave exactly as every developer wants it to in every single use case is virtually impossible. For instance - I have used tabs many times and never had to calculate height. So this is more an issue of the way you are using it, rather than a problem with the component itself (In my opinion).

That said, if it is a problem with the component, then keep this open and hopefully the team will address it at some point.

@ccurtis0
Copy link

I have a similar but opposite problem - I also want to calculate the height, but I want to know the height with just the old content shown. The current 'onShow' signature is:

.onShow [this=ul.tabs] ( newContentDiv )

I would like it to be:

.onShow [this=ul.tabs] ( newContentDiv, oldContentDiv )

I believe this would also solve your problem, @philipraets , because for your needs the delta is simply the current height - (newContent.offsetHeight - oldContent.offsetHeight). Knowing which tab is active isn't immediately helpful because you still have to look up the content elements yourself.

Personally, I prefer the current state with both elements having the class 'active' because you can get the intended-active tab via either this.$activeTabLink or this.$tabLinks in the onShow handler.

Agree?

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

No branches or pull requests

3 participants