Server side rendering for scrolled entries

[tf]

REDMINE-17566

[coveralls]

Coverage remained the same at 99.191% when pulling 80d25f9 on tf:ssr into 1cb852d on codevise:master.

[aviav]

Depending on your view on this, I could also directly merge this.

I haven't checked anything in the browser yet -- this I would normally do once the feedback is dealt with. I'll take a look at staging now, since I didn't spot major flaws, but while I do that, you could already have a look at the review

[aviav]

suggestion(if-minor): The caller of this method also makes the same case distinction. Maybe it would make sense to split the branches into different methods now. Related to question above

[tf]

Both times (edit mode or not) the function is called from one the same place [1, 2]. Detecting edit mode there seems worse.

I tried inlining render, but it also did not make things clearer IMO.

[aviav]

To clarify: The caller of render would be pageflowScrolledRender, and in both of these methods, we have a branching against the same editMode variable. This means that we'd really only need to branch once, which would be more streamlined

[aviav]

What I just described is what you mean by inlining render, right? In case you think it'd make the code worse, let's leave it like this

[tf]

What I tried was:

global.pageflowScrolledRender = function(seed) {
  setupI18n(seed.i18n);

  browser.detectFeatures().then(function(){
    const rootElement = document.getElementById('root');
    const root = <Root seed={seed} />;

    if (editMode) {
      loadInlineEditingComponents().then(() =>
        ReactDOM.render(root, rootElement)
      );
    }
    else {
      ReactDOM.hydrate(root, rootElement);
    }
  });
}

This only branches once. Still, I do not like it better. Duplicating the conditional actually makes the difference stick out more for me: Once it's about using render vs hydrate, the other time about loading inline editing components or not.

[aviav]

I'll not impose my opinion here. Both ways, the PR as is brings a meaningful improvement. I like one way better, you like the other way better.

My preference is mostly based on the feeling that I might be missing an aspect of the branching logic that might lead to combinations like either first true branch then false branch or the other way around. Having the code as in your previous comment makes it clearer that true then true and false then false are the only possible cases, and we don't need to look out for side effects that suddenly toggle editMode.

I see how the exact same indentation of the ReactDOM API usages can provide another sort of clarity