Skip to content

Commit 38290ea

Browse files
castastrophegithub-actions[bot]castastrophe
authored
fix: Updating pfelement to cascade on attribute; add back persist test (#1554)
* fix: Updating pfelement to cascade on attribute * fix: Bring back comments removed and tidy up parent selector * fix: Update which function is run for on attribute rendering * fix: Update changelog * fix: Add check for parentNode * fix: Switch back to parentNode * fix: Fixing resetContext logic * fix: Switch content set to use resetContext and remove console outputs * fix: Remove duplicate isPFElement * fix: Remove isPFElement Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: castastrophe <[email protected]>
1 parent ef9d6ef commit 38290ea

File tree

6 files changed

+69
-29
lines changed

6 files changed

+69
-29
lines changed

CHANGELOG-1.x.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
# 1.6.0 (2021)
1+
# 1.6.0 (2021-04-23)
22

3-
- [](https://github.com/patternfly/patternfly-elements/commit/) feat: JSDoc preview added for PFElement
3+
- [0a549f8](https://github.com/patternfly/patternfly-elements/commit/0a549f8c54037e01006063800e729d633b515f66) feat: JSDoc preview added for PFElement
44
- [65983e6](https://github.com/patternfly/patternfly-elements/commit/65983e60d5394116d3dce6870b77f72772fa09c0) fix: pfe-primary-detail IE11 rendering fix
5+
- [](https://github.com/patternfly/patternfly-elements/commit/) fix: Context initialization cascading from parent to child (#1438)
56

67
# 1.5.1 (2021-04-15)
78

elements/pfe-accordion/demo/index.html

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,12 @@
4646
<h2>Accordion component</h2>
4747
</header>
4848
<article>
49-
<pfe-accordion>
49+
<pfe-accordion id="accordion-default">
5050
<pfe-accordion-header id="first-header">
5151
<h3>Nested panel example</h3>
5252
</pfe-accordion-header>
5353
<pfe-accordion-panel>
54-
<pfe-accordion disclosure="false">
54+
<pfe-accordion disclosure="false" id="accordion-nested">
5555
<pfe-accordion-header>
5656
<h3>Panel with multiple paragraphs</h3>
5757
</pfe-accordion-header>
@@ -107,12 +107,12 @@ <h3>Panel with CTA</h3>
107107
<h2>Accordion component on dark</h2>
108108
</header>
109109
<article>
110-
<pfe-accordion>
110+
<pfe-accordion id="accordion-darkest">
111111
<pfe-accordion-header>
112112
<h3>Nested panel example</h3>
113113
</pfe-accordion-header>
114114
<pfe-accordion-panel>
115-
<pfe-accordion>
115+
<pfe-accordion id="accordion-darkest-nested">
116116
<pfe-accordion-header>
117117
<h3>Panel with multiple paragraphs</h3>
118118
</pfe-accordion-header>

elements/pfe-content-set/src/pfe-content-set.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,8 @@ class PfeContentSet extends PFElement {
483483
Promise.all([customElements.whenDefined(tag)]).then(() => {
484484
this.cascadeProperties();
485485

486+
this.resetContext();
487+
486488
// Attach the mutation observer
487489
if (!this.isIE11) this._observer.observe(this, CONTENT_MUTATION_CONFIG);
488490

elements/pfe-content-set/test/pfe-content-set_test.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,10 @@ suite('<pfe-content-set>', () => {
215215

216216
test("it should set the correct \"on\" attribute from a parent component that has a color attribute to the nested tabs",
217217
done => {
218-
flush(() => {
219-
const tabBand = document.querySelector("#band");
220-
const contentSet = tabBand.querySelector("pfe-content-set");
218+
const tabBand = document.querySelector("#band");
219+
const contentSet = tabBand.querySelector("pfe-content-set");
221220

221+
Promise.all([customElements.whenDefined("pfe-content-set")]).then(() => {
222222
const tabs = contentSet.view;
223223
const tab = tabs.querySelector("pfe-tab");
224224
const panel = tabs.querySelector("pfe-tab-panel");
@@ -236,8 +236,8 @@ suite('<pfe-content-set>', () => {
236236

237237
done();
238238
});
239-
240239
});
240+
241241
});
242242

243243
test("it should set the correct \"on\" attribute from a parent component that has a color attribute to the nested accordion",

elements/pfelement/src/pfelement.js

Lines changed: 47 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -285,22 +285,52 @@ class PFElement extends HTMLElement {
285285
* This alerts nested components to a change in the context
286286
*/
287287
contextUpdate() {
288-
// If a value has been set, alert any nested children of the change
289-
[...this.querySelectorAll("*"), ...this.shadowRoot.querySelectorAll("*")]
288+
// Loop over light DOM elements, find direct descendants that are components
289+
const lightEls = [...this.querySelectorAll("*")]
290290
.filter(item => item.tagName.toLowerCase().slice(0, 4) === `${prefix}-`)
291-
.map(child => {
292-
this.log(`Update context of ${child.tag}`);
293-
Promise.all([customElements.whenDefined(child.tagName.toLowerCase())]).then(() => {
294-
// Ask the component to recheck it's context in case it changed
295-
child.resetContext(this.on);
296-
});
291+
// Closest will return itself or it's ancestor matching that selector
292+
.filter(item => {
293+
// If there is no parent element, return null
294+
if (!item.parentElement) return;
295+
// Otherwise, find the closest component that's this one
296+
else return item.parentElement.closest(`[${this._pfeClass._getCache("prop2attr").pfelement}]`) === this;
297+
});
298+
299+
// Loop over shadow elements, find direct descendants that are components
300+
let shadowEls = [...this.shadowRoot.querySelectorAll("*")]
301+
.filter(item => item.tagName.toLowerCase().slice(0, 4) === `${prefix}-`)
302+
// Closest will return itself or it's ancestor matching that selector
303+
.filter(item => {
304+
// If there is a parent element and we can find another web component in the ancestor tree
305+
if (item.parentElement && item.parentElement.closest(`[${this._pfeClass._getCache("prop2attr").pfelement}]`)) {
306+
return item.parentElement.closest(`[${this._pfeClass._getCache("prop2attr").pfelement}]`) === this;
307+
}
308+
// Otherwise, check if the host matches this context
309+
if (item.getRootNode().host === this) return true;
310+
311+
// If neither state is true, return false
312+
return false;
297313
});
314+
315+
const nestedEls = lightEls.concat(shadowEls);
316+
317+
// If nested elements don't exist, return without processing
318+
if (nestedEls.length === 0) return;
319+
320+
// Loop over the nested elements and reset their context
321+
nestedEls.map(child => {
322+
this.log(`Update context of ${child.tagName.toLowerCase()}`);
323+
Promise.all([customElements.whenDefined(child.tagName.toLowerCase())]).then(() => {
324+
// Ask the component to recheck it's context in case it changed
325+
child.resetContext(this.on);
326+
});
327+
});
298328
}
299329

300330
resetContext(fallback) {
301331
if (this.isIE11) return;
302332

303-
this.log(`Resetting context on ${this.tag}`);
333+
this.log(`Resetting context`);
304334
// Priority order for context values to be pulled from:
305335
//--> 1. context (OLD: pfe-theme)
306336
//--> 2. --context (OLD: --theme)
@@ -429,8 +459,8 @@ class PFElement extends HTMLElement {
429459
// Cascade properties to the rendered template
430460
this.cascadeProperties();
431461

432-
// Reset the display context
433-
this.resetContext();
462+
// Update the display context
463+
this.contextUpdate();
434464

435465
if (PFElement.trackPerformance()) {
436466
try {
@@ -494,7 +524,7 @@ class PFElement extends HTMLElement {
494524
const cascade = this._pfeClass._getCache("cascadingProperties");
495525

496526
if (cascade) {
497-
if (window.ShadyCSS && this._cascadeObserver) this._cascadeObserver.disconnect();
527+
if (this._cascadeObserver) this._cascadeObserver.disconnect();
498528

499529
let selectors = Object.keys(cascade);
500530
// Find out if anything in the nodeList matches any of the observed selectors for cacading properties
@@ -525,7 +555,7 @@ class PFElement extends HTMLElement {
525555
}
526556

527557
// @TODO This is here for IE11 processing; can move this after deprecation
528-
if (window.ShadyCSS && this._rendered && this._cascadeObserver)
558+
if (this._rendered && this._cascadeObserver)
529559
this._cascadeObserver.observe(this, {
530560
attributes: true,
531561
childList: true,
@@ -550,6 +580,7 @@ class PFElement extends HTMLElement {
550580
*/
551581
_contextObserver(oldValue, newValue) {
552582
if (newValue && ((oldValue && oldValue !== newValue) || !oldValue)) {
583+
this.log(`Running the context observer`);
553584
this.on = newValue;
554585
this.cssVariable("context", newValue);
555586
}
@@ -560,6 +591,7 @@ class PFElement extends HTMLElement {
560591
*/
561592
_onObserver(oldValue, newValue) {
562593
if ((oldValue && oldValue !== newValue) || (newValue && !oldValue)) {
594+
this.log(`Context update`);
563595
// Fire an event for child components
564596
this.contextUpdate();
565597
}
@@ -593,8 +625,6 @@ class PFElement extends HTMLElement {
593625
if (mutation.type === "childList" && mutation.addedNodes.length) {
594626
this.cascadeProperties(mutation.addedNodes);
595627
}
596-
// @TODO: Do something when mutation type is attribute?
597-
// else if (mutation.type === "attributes") {}
598628
}
599629
}
600630
/* --- End observers --- */
@@ -693,7 +723,7 @@ class PFElement extends HTMLElement {
693723
_initializeSlots(tag, slots) {
694724
this.log("Validate slots...");
695725

696-
if (window.ShadyCSS && this._slotsObserver) this._slotsObserver.disconnect();
726+
if (this._slotsObserver) this._slotsObserver.disconnect();
697727

698728
// Loop over the properties provided by the schema
699729
Object.keys(slots).forEach(slot => {
@@ -739,7 +769,7 @@ class PFElement extends HTMLElement {
739769

740770
this.log("Slots validated.");
741771

742-
if (window.ShadyCSS && this._slotsObserver) this._slotsObserver.observe(this, { childList: true });
772+
if (this._slotsObserver) this._slotsObserver.observe(this, { childList: true });
743773
}
744774

745775
/**

scripts/test.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,21 @@ const argv = require("yargs")
1212
["npm test", "(run all tests, builds by default)"],
1313
["npm test -- pfe-card", "(run one test)"],
1414
["npm test -- pfe-card pfe-band", "(run multiple tests)"],
15-
["npm test -- --nobuild", "(do not build the component(s) prior to running tests)"]
15+
["npm test -- --nobuild", "(do not build the component(s) prior to running tests)"],
16+
["npm test -- --debug", "(maintain the test environment for debugging)"]
1617
])
1718
.options({
1819
nobuild: {
1920
default: false,
2021
alias: "nb",
2122
describe: "do not build the component(s) prior to running tests",
2223
type: "boolean"
24+
},
25+
debug: {
26+
default: false,
27+
alias: "p",
28+
describe: "maintain the test environment for debugging",
29+
type: "boolean"
2330
}
2431
}).argv;
2532

@@ -35,6 +42,6 @@ const build = !argv.nobuild ? `npm run build ${components.join(" ")} && ` : "";
3542
shell.exec(
3643
`${build}./node_modules/.bin/wct --config-file wct.conf.json --npm ${
3744
components ? components.map(item => `\"./elements/${item}/test\"`).join(" ") : ""
38-
}`,
45+
}${argv.debug ? " -p" : ""}`,
3946
code => process.exit(code)
4047
);

0 commit comments

Comments
 (0)