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.

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

refactor: simplify observers setup, improve tests #8622

Merged
merged 5 commits into from
Feb 4, 2025

Conversation

vursen
Copy link
Contributor

@vursen vursen commented Feb 3, 2025

Description

The PR simplifies the MutationObserver setup in FormLayout by using subtree: true instead of observing each child element individually. The PR also improves the tests to ensure they cover these observers. Previously, the tests were passing even without the observers because adding and removing new elements triggered a resize event, which in turn triggered a layout update expected by the test.

Depends on

Part of #8583

Type of change

  • Refactor

@@ -598,32 +581,25 @@ describe('form layout', () => {
expect(unhiddenItemWidth).to.equal(itemWidth);
});

it('should update layout after updating a colspan attribute on the lazily stamped node', async () => {
container.push('items', { label: 'Email', colspan: 1 });
Copy link
Contributor Author

@vursen vursen Feb 3, 2025

Choose a reason for hiding this comment

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

note: It seems pointless to test that FormLayout picks up on DOM changes caused by Polymer specifically.

@vursen vursen marked this pull request as ready for review February 3, 2025 12:12

/** @private */
_getObservableNodes(nodeList) {
const ignore = ['template', 'style', 'dom-repeat', 'dom-if'];
Copy link
Contributor Author

@vursen vursen Feb 3, 2025

Choose a reason for hiding this comment

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

note: This seems like an old logic to support Polymer templates, which we no longer support, as far as I remember.

@vursen vursen force-pushed the refactor/form-layout/simplify-observers-and-tests branch from 64027f0 to 15137a6 Compare February 3, 2025 12:44
Copy link

sonarqubecloud bot commented Feb 4, 2025

@vursen vursen merged commit a69ec93 into main Feb 4, 2025
9 checks passed
@vursen vursen deleted the refactor/form-layout/simplify-observers-and-tests branch February 4, 2025 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants