matestack / matestack-ui-core

Component based web UIs in pure Ruby for Rails. Boost your productivity & easily create component based web UIs in pure Ruby.
https://www.matestack.io
MIT License
547 stars 47 forks source link

Issue 549 #571

Open meaagan opened 2 years ago

meaagan commented 2 years ago

Issue https://github.com/matestack/matestack-ui-core/issues/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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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!

jonasjabari commented 2 years ago

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:

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"