Issue 549

[meaagan]

Issue #549: hide_after should work without show_on: 'event'

Hi! As you can see, this PR only consists of a bunch of console.logs and puts. I did this to show where I was looking for the bug/solution and to potentially get a hint on where to start. I was (obviously) unsuccessful, but I am hoping that I was looking at the correct lines or at least in the right files.

I understand that the issue comes from the mount as it says in the description of the bug (the lifecycle is never started, therefore the timer also never starts), but knowing that it comes from a vue.js file, I could not figure out what to do.

[pascalwengerter]

Hey, thanks for opening this PR! I'm not overly familiar with the codebase anymore, but from what I've gathered from a quick skimming, could you try the following in https://github.com/matestack/matestack-ui-core/blob/19b5b8c97ee551d8d2f80840aa2d1c16a5727e9e/lib/matestack/ui/vue_js/components/toggle.js ?

Put this code snippet from the show method

      if(this.props["hide_after"] != undefined){
        self.hide_after_timeout = setTimeout(function () {
          self.hide()
        }, parseInt(this.props["hide_after"]));
      }

into the created function of the toggle component?

Also, line 46-48 of this component can be safely deleted from my POV since they already get covered in line 37-38

[pascalwengerter]

Elaboration: The way I read the issue, the toggle component misses some sort of awareness about its hide_after configuration, and since all the other props get checked in the created function I came up with the reasoning above. Maybe @jonasjabari can take a look and clarify?

[meaagan]

@pascalwengerter Wow thanks for commenting so quickly! I tried out your solution with no success last night, but I do think it's close. I tried a few other things based on your solution with again, no success, but I will keep trying. Thanks again!

[pascalwengerter]

Maybe I need to setup Ruby/Rails/matestack on my machine as well to get my hands dirty on this 👨🏽‍🔬 just some quick review of your changes since I got the notification

[pascalwengerter]

I don't think you'll have to touch this component though, the hide method is already called in a timeout in line 35 if hide_after is defined

[jonasjabari]

Yes this component does not need to be touched. The hide things in the async component should actually be deleted at some point...

[pascalwengerter]

No need to rename this from my POV, but if you want to then please do so in the rest of the file as well ;)

[jonasjabari]

Hey @meaagan @pascalwengerter!

Sorry for my late reply on this one! And thanks a lot @meaagan for your PR! I agree with @pascalwengerter suggestion:

Put this code snippet from the show method

      if(this.props["hide_after"] != undefined){
        self.hide_after_timeout = setTimeout(function () {
          self.hide()
        }, parseInt(this.props["hide_after"]));
      }

into the created function of the toggle component?

Also, line 46-48 of this component can be safely deleted from my POV since they already get covered in line 37-38

show_on and hide_after should work independently from each other:

• Using only show_on should show the content on an event

• Using shown_on and hide_after together should (like currently working) show the content on an event and from then on start a timer in order to hide the content again after the specified time

• Using only hide_after should hide the content after mount in a specified time

With the suggested addition of @pascalwengerter, a combination of show_on and hide_after would work. The content is not visible at the beginning and the hider_after timer is started. After the given time, the timer would trigger hiding the content, but it's already hidden. That's ok IMO. The time is then again triggered when the specified event is emitted. So that behavior would still work. But we should add a clearTimeout (the one from the destroy hook) every time the show event is emitted. Otherwise the initial timer triggered on mount and the one triggered from the show method might "conflict"