Start time ago loops

In pipelines store there is a function that start a loop of a component and requires a specific structure and it's the cause of many problems. This function needs to be moved inside the component.

mentioned in issue #31155 (moved)

mentioned in issue #31349 (closed)

assigned to @filipa

This is the component https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/pipelines/components/time_ago.js

When we receive data we call the changetime function every 10000 seconds.

These are my findings:

• tooltip not being set with title
• this options object is never used https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/pipelines/components/time_ago.js#L21
• change time is always true: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/pipelines/components/time_ago.js#L29
• this returns an object with the finishedTime formated: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/pipelines/components/time_ago.js#L30
• this property is never used https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/pipelines/components/time_ago.js#L50

I think it's safe to remove all the above code not being used and either remove this call to changetime or actually change the time.

I was going to move this to use Visibility.js instead of Vue realtime listener: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/vue_realtime_listener but I don't think this is doing anything at all

@iamphill @brycepj can you help me take a look? I can be seeing this wrong.

Which makes these call not needed: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/pipelines/stores/pipelines_store.js#L43

@filipa you're right, looks like there's some unused code. A couple comments:

tooltip not being set with title

Do the tooltips actually work or no? I'm thinking they should still work because data-original-title is the attribute bootstrap uses internally to track the title. The title attribute is just used to initialize the tooltip, IIRC.

So if you want to update a tooltip dynamically, you have to update the data-original-title attr. Probably wouldn't hurt to set the title attribute for clarity and readability, but it probably doesn't need to be set for tooltips to work.

change time is always true: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/pipelines/components/time_ago.js#L29

That seems right.

this property (changeTime) is never used https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/pipelines/components/time_ago.js#L50

Looks like changeTime is called here

@brycepj Thanks for the help! The method is called, but the this.currentTime prop doesn't do anything, it's used in an if and it's always true, isn't it?

Regarding tooltip, according to the documentation I think we need to set the title and not data-original-title.

In order to properly update the bootstrap tooltip, we need to destroy it and mount it again in order to update it properly, since we are updating a virtual DOM and bootstrap works on the actual DOM. Me and @fatihacet discussed this not that long ago I believe, what is the MR where we added this change, do you know @fatihacet ?

changed milestone to %9.2

mentioned in merge request !10897 (merged)

Regarding tooltip, according to the documentation I think we need to set the title and not data-original-title. In order to properly update the bootstrap tooltip

We're already using data-original-title in many other places. I like it as a solution because it:

1. allows us to keep $.tooltip out of our Vue components
2. it requires no extra code
3. keeps the data flow in our components unidirectional

One other important concern is that on pages with lots of timeagos, using data-original-title means you get to skip running all the initialization and destruction code for every update of every timeago element. That CPU required for large numbers of updates can really add up.

In order to properly update the bootstrap tooltip, we need to destroy it and mount it again in order to update it properly, since we are updating a virtual DOM and bootstrap works on the actual DOM.

I believe Bootstrap just listens for delegated events (registered on document). So as long as the virtual DOM has been applied, Bootstrap can use it.

@filipa Is the best option to re-write it? It seems like an overly complex way to just update some times & if we don't really understand what is happening, then we should really do it a better way.

Yes we totally should use Visibility.js instead of https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/vue_realtime_listener/index.js. We need to stop these random things getting in & stick to how our documentation states things should be done.

@brycepj We shouldn't update data-original-title, we can't guarantee that Bootstrap will always use that. The best way is to update the title attribute & then call fixTitle on the element. Sure Bootstrap just changes the data-original-title but this is the correct way. We can even run that on a per element basis $(el).tooltip('fixTitle')

@iamphill thanks for the help! We know the intention: this was supposed to start a timer, but it doesn't.

Since we already have realtime, there is no need for this anymore.

Regarding the tooltip: me and @fatihacet noticed fixTitle is not enough we add to go with destroying it and initialising it again.

We should think of writing a vue component for tooltips

Do you know fixTitle wasn't enough? It should be

Do you know fixTitle wasn't enough? It should be

No. @fatihacet do you know? I don't remember, you tried it locally, correct?

mentioned in merge request !10718 (merged)

mentioned in issue #29395 (closed)

closed via merge request !10897 (merged)

mentioned in commit 5c195199