lion-step renders the element even though .condition is false

[takrishna]

Expected behavior

.condition - Dynamic condition, when true the step is added to the flow.

ing-step should not render the element inside it unless both of the below conditions are true:

1. The step is reached (i.e. visible to the user)
2. .condition (flag) is evaluated to true.

Actual Behavior

In actuality even when .condition is false - the step is getting added to the DOM and simply skipped at the time of navigation. If we have properties which are dynamically generated (e.g, by calling a function) for a component defined inside the ing-step they get invoked and cause unwanted side-effects. ing-step must not render element inside it until the step is reached and also condition is evaluated to true.

Updated code sample (be prepared to see two annoying alert messages :P)

https://webcomponents.dev/edit/EeIjuQ4dTHZrr2l1OpV0

Additional context

Let me know if you need more info

[jorenbroekema]

So, I think for the most part this behavior is intended, but I understand that rendering a skipped step may have unwanted side effects.

Instead of always rendering all steps and CSS hiding the ones that should be hidden, we could potentially only render the current step.

However, for other components such as combobox, tabs, collapsible, you could make the same argument. In general I think it's better to use CSS hide/show in these cases, including for steps. I would probably advise you not to run side effects as part of being rendered to DOM, but run these side effects as a consequence of being "entered".

updated(changedProperties) {
  if (changedProperties.get('status') === 'entered') {
    doSideEffect();
  }
}

[takrishna]

I think if tabs are currently behaving the same way perhaps that could be modified as well - here is an example from another popular library where contents of the tab do not get rendered unless the tab is selected- Angular Material Tab.

1. We have about 50+ lion-steps in a flow, which means when the flow bootstraps 50+ components would un-wantedly bootstrap (go through the start up lifecycle). Firing up chrome inspector on performance is colorful 🗡️
2. Regarding "run these side effects as a consequence of being "entered" ('status') === 'entered'):
   = This doesn't prevent the above problem (point 1)
   = Since certain properties are inflated only after a step is reached (e.g., say in a 10 step flow a property could be set in step 5 to be used in step 7). We have to ensure in Step 7 enough null safety/protection is provided (say render nothing when a certain required property is missing) Which will simply get re-rendered when the step is reached (user never sees this at all. All these can get tideous.

[MathieuPuech]

I just have a discussion related to this topic on slack: https://polymer.slack.com/archives/C03PF4L4L/p1615419006097500

the issue is that you can’t do that with custom elements currently. It could be done with custom templating or templating with lit-html TemplateResult.

[jorenbroekema]

Nice discussion, do you think it would make sense if I provided a similar helper in lion for enabling this? similar to https://github.com/microsoftgraph/microsoft-graph-toolkit/blob/main/packages/mgt-element/src/utils/TemplateHelper.ts or https://www.npmjs.com/package/stampino? Or just leave this up to subclassers to override render and figure this out on their end?

[MathieuPuech]

I suppose we could try to do something similar. I don't know if using <template> will solve the issue but we can see it solve it