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

Identifying compositions by handler URI #123

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Identifying compositions by handler URI #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5660b5f
Added changes that referes to latest changes proposed in the blending…
PatrykSzwer Apr 24, 2019
673a780
revert the check for the existance of compositionProvider.ViewUris
warpech Apr 24, 2019
411a40a
reflect the JSON structure of BlendingProvide 8.1.0 in the test fixture
warpech Apr 24, 2019
401447f
Removed conditional set of defaultViewHtmlsToBeRendered and replaced …
PatrykSzwer Apr 25, 2019
63a32d4
Merge branch 'master' into handler-uri-as-composition-identifier
warpech Apr 26, 2019
e7a9b1e
Fixed part of the tests by adding ViewUris and ActualViewUris to the …
PatrykSzwer Apr 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 23 additions & 20 deletions starcounter-include.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,27 +310,30 @@
return this._renderCompositionChange(compositionProviderComposition, () => {
// convert string from view-model, to actual document fragment
const customComposition = this.stringToDocumentFragment(compositionProviderComposition);

// append default compositions for views not covered by this composition
if (compositionProvider.ViewUris) {
const keys = Object.keys(this.viewModel);
for(let key of keys) {
const scoped = this.viewModel[key];
if (scoped && scoped.Html && compositionProvider.ViewUris.indexOf(scoped.Html) === -1) {
if (this.defaultComposition) {
this.template.scopedNodes.forEach((nodes) => {
if(nodes.scope === key) {
nodes.forEach((node) => appendComposition(customComposition, node));
}
});
}
else {
// BUG,TODO (tomalec): looks like a bug
// This means that if there *is* a custom composition, but it does not cover all views,
// and it happend that none of the views had a default composition,
// we would discard custom composition and use default -> fallback
return false;
}
const defaultViewHtmlsToBeRendered =
compositionProvider.ActualViewUris.filter(
x => !compositionProvider.ViewUris.includes(x.hasOwnProperty("BlendingUri") ? x.BlendingUri : x))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test for the path that contain BlendingUri?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today is @PatrykSzwer's last day, so he wont add anything but I may.

Can you add a test for the path that contain BlendingUri?

This whole section never had a single test. This PR does not make it much worse, but I agree tests should be added.

starcounter-include/starcounter-include.html

Lines 315 to 339 in e7a9b1e

const defaultViewHtmlsToBeRendered =
compositionProvider.ActualViewUris.filter(
x => !compositionProvider.ViewUris.includes(x.hasOwnProperty("BlendingUri") ? x.BlendingUri : x))
.map(x => x.hasOwnProperty("ViewHtml") ? x.ViewHtml : x);
const keys = Object.keys(this.viewModel);
for (let key of keys) {
const scoped = this.viewModel[key];
if (scoped && scoped.Html && defaultViewHtmlsToBeRendered.includes(scoped.Html)) {
if (this.defaultComposition) {
this.template.scopedNodes.forEach((nodes) => {
if (nodes.scope === key) {
nodes.forEach((node) => appendComposition(customComposition, node));
}
});
}
else {
// BUG,TODO (tomalec): looks like a bug
// This means that if there *is* a custom composition, but it does not cover all views,
// and it happend that none of the views had a default composition,
// we would discard custom composition and use default -> fallback
return false;
}
}
}

.map(x => x.hasOwnProperty("ViewHtml") ? x.ViewHtml : x);

const keys = Object.keys(this.viewModel);
for (let key of keys) {
const scoped = this.viewModel[key];
if (scoped && scoped.Html && defaultViewHtmlsToBeRendered.includes(scoped.Html)) {
if (this.defaultComposition) {
this.template.scopedNodes.forEach((nodes) => {
if (nodes.scope === key) {
nodes.forEach((node) => appendComposition(customComposition, node));
}
});
}
else {
// BUG,TODO (tomalec): looks like a bug
// This means that if there *is* a custom composition, but it does not cover all views,
// and it happend that none of the views had a default composition,
// we would discard custom composition and use default -> fallback
return false;
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions test/composition/default-declarative-shadow-dom.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ <h2>Custom Shadow DOM alternative composition</h2>
"CompositionProvider_0": {
"PartialId": "given PartialId",
"Composition": "",
"ViewUris": []
"ViewUris": [],
"ActualViewUris": ["template_w_declarative-shadow-dom-alternative.html"]
},
"App": {
"Html": "template_w_declarative-shadow-dom-alternative.html",
Expand All @@ -76,7 +77,8 @@ <h2>Custom Shadow DOM alternative composition</h2>
"CompositionProvider_0": {
"PartialId": "given PartialId",
"Composition": "custom!",
"ViewUris": ["template_w_declarative-shadow-dom.html"]
"ViewUris": ["template_w_declarative-shadow-dom.html"],
"ActualViewUris": ["template_w_declarative-shadow-dom.html", "template_w_declarative-shadow-dom-alternative.html"]
},
"App1": {
"Html": "template_w_declarative-shadow-dom.html",
Expand Down
6 changes: 4 additions & 2 deletions test/composition/parent-declarative-shadow-dom.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ <h2>Custom Shadow DOM alternative composition</h2>
"CompositionProvider_0": {
"PartialId": "given PartialId",
"Composition": "",
"ViewUris": []
"ViewUris": [],
"ActualViewUris": ["template_w_declarative-shadow-dom-alternative.html"]
},
"App": {
"Html": "template_w_declarative-shadow-dom-alternative.html",
Expand All @@ -84,7 +85,8 @@ <h2>Custom Shadow DOM alternative composition</h2>
"CompositionProvider_0": {
"PartialId": "given PartialId",
"Composition": "custom!",
"ViewUris": ["template_w_declarative-shadow-dom.html"]
"ViewUris": ["template_w_declarative-shadow-dom.html"],
"ActualViewUris": ["template_w_declarative-shadow-dom.html", "template_w_declarative-shadow-dom-alternative.html"]
},
"App1": {
"Html": "template_w_declarative-shadow-dom.html",
Expand Down
6 changes: 4 additions & 2 deletions test/view-model-attribute/cleanup_removed_partial_Layout.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<!doctype html>
<!doctype html>
<html>

<head>
Expand Down Expand Up @@ -36,7 +36,9 @@
"CompositionProvider_0": {
"Html": "../templateToInclude.html",
"Composition": 'a Composition',
"PartialId": "given PartialId"
"PartialId": "given PartialId",
"ViewUris": [""],
"ActualViewUris": [""]
},
"doesItWork": "works!"
};
Expand Down
6 changes: 4 additions & 2 deletions test/view-model-attribute/nested-html/stamped_from_polymer_template.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<!doctype html>
<!doctype html>
<html>

<head>
Expand Down Expand Up @@ -42,7 +42,9 @@
},
'CompositionProvider_0': {
'Composition': 'custom composition',
'doesItWork': 'works!'
'doesItWork': 'works!',
"ViewUris": [""],
"ActualViewUris": [""]
}
};
};
Expand Down