Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use starcounter-include in default PartialToStandaloneHtmlProvider #89

Closed
tomalec opened this issue Apr 20, 2017 · 9 comments
Closed

Use starcounter-include in default PartialToStandaloneHtmlProvider #89

tomalec opened this issue Apr 20, 2017 · 9 comments
Assignees
@chrhol

Comments

@tomalec
Copy link

tomalec commented Apr 20, 2017

Currently default PartialToStandaloneHtmlProvider uses just imported-template to stamp root partial:

<body>
    <template is=""dom-bind"" id=""puppet-root"">
        <template is=""imported-template"" content$=""{{{{model.Html}}}}"" model=""{{{{model}}}}""></template>
    </template>
     ...

It results in lack of support for any layout features, like default declarative-shadow-dom (eventually could be fixed by https://github.com/Starcounter/RebelsLounge/issues/78) and explicit Shadow DOM compositions (ones from Starcounter.HTMLComposition given by CompositionProvider).

That's why I would suggest to

  1. Use starcounter-include on root level: 
    <body>
    <starcounter-include view-model=""{{{{model}}}}""></starcounter-include>
    ...
    As we didn't deprecate non-namespaced view-models yet (Drop support for non-namespaced partial views & view-models starcounter-include#29) it should work out of the box. But given that we decided to deprecate it eventually, it would require to have namespaced view-model JSON on root level. @chrhol is that possible, easy enough to implement?

Alternatively we could

  1. Rely on declarative-shadow-dom as independent custom element (https://github.com/Starcounter/RebelsLounge/issues/78 once it's done) . Then default ones (provided by app devs - from app code) should work regardless of the container. Then the explicit custom ones, given by the composer/editor/designer app/person (ones from the database) could be delivered also using <declarative-shadow-dom> merged by HTML merger.
    However, this would require:
    1. Implementing <d-s-d> as independent CE which may take some time and is at risk due to lack of is="" support in browsers and other potential lifecycle problems,
    2. Heavy re-design in composition flow (which I planned anyways, but in long-term),
    3. Changes to HTML Merger, which was pretty stable so far,
    4. Implementing additional feature to <d-s-d> to overwrite instead of appending Shadow DOM
    5. Workaround in some other way the problem of missing slots and slot names (current responsibility of <starcounter-include>/<juicy-composition>
  2. Suggest to all app developers, to use MaterPage.html - which we already found confusing.

/cc @warpech @Mackiovello
@warpech
Copy link

warpech commented Apr 20, 2017

  1. Looks the best to me. It goes on pair with https://github.com/Starcounter/level1/issues/4017.

@warpech
Copy link

warpech commented Apr 24, 2017

Ping @chrhol

@Mackiovello Mackiovello mentioned this issue Apr 28, 2017
Closed
@warpech
Copy link

warpech commented May 5, 2017

Ping @chrhol. This is very much needed

@chrhol
Copy link

chrhol commented May 10, 2017

So the tasks are to:

  • Update PartialToStandaloneHtmlProvider to use starcounter-include
  • Change so that root json is namespaced.

Is that correct, or is the second task optional? I feel a little confused :) And there is no other things that needs to be changed, so although behaviour is changing, it shouldn't break any apps?

@chrhol chrhol self-assigned this May 10, 2017
@warpech
Copy link

warpech commented May 10, 2017

Yes, the beginning might be as easy as changing

<template is=""imported-template"" content$=""{{{{model.Html}}}}"" model=""{{{{model}}}}""></template>

to

<starcounter-include view-model=""{{{{model}}}}""></starcounter-include>

@tomalec perhaps you could make a PR to Starcounter that does that?

The intent to implement Starcounter/starcounter-include#29 in starcounter-include requires https://github.com/Starcounter/level1/issues/4017 to be implemented in server-side first, but that one is already tracked.

I tried to check if there are any more challenges, but could not find such.

@tomalec
Copy link
Author

tomalec commented May 10, 2017 •
edited by warpech
Loading

Yes, the beginning might be as easy as changing

But isn't it just maintaining one hack just to hide a problem?

Starcounter/starcounter-include#29 in starcounter-include requires Starcounter/level1#4017

Does it? why? I think currently it doesn't and I would say using sc-include on root level requires it

@warpech
Copy link

warpech commented May 11, 2017 •
edited
Loading

But isn't it just maintaining one hack just to hide a problem?

What do you mean by hack? This is exactly what's is asked in the title of this issue.

Does it? why? I think currently it doesn't and I would say using sc-include on root level requires it

Sorry, that was a mental shortcut. What I meant was: If we put starcounter-include to PartialToStandaloneHtmlProvider right now, then Starcounter/starcounter-include#29 must be postponed until Starcounter/level1#4017 is done. That is fine for me.

@tomalec tomalec mentioned this issue May 29, 2017
Closed
@tomalec
Copy link
Author

tomalec commented Jun 2, 2017

Solved as suggested above by https://github.com/Starcounter/level1/issues/4223

@tomalec tomalec closed this as completed Jun 2, 2017
@warpech
Copy link

warpech commented Jun 5, 2017

@tomalec will you please write an announcement about this change? https://github.com/Starcounter/Announcements

@warpech warpech mentioned this issue Jun 6, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chrhol chrhol

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tomalec @warpech @chrhol