Skip to content

Add Element.toggleAttribute() polyfill and support in Shady DOM#541

Merged
justinfagnani merged 23 commits into
masterfrom
toggle-attribute
Mar 29, 2023
Merged

Add Element.toggleAttribute() polyfill and support in Shady DOM#541
justinfagnani merged 23 commits into
masterfrom
toggle-attribute

Conversation

@justinfagnani

@justinfagnani justinfagnani commented Mar 23, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

No description provided.

@justinfagnani justinfagnani mentioned this pull request Mar 24, 2023
Merged
bicknellr
bicknellr reviewed Mar 24, 2023
View reviewed changes
Comment thread packages/custom-elements/ts_src/Patch/Element.ts Outdated
Comment thread packages/custom-elements/ts_src/Patch/Element.ts Outdated
Comment thread packages/shadydom/src/wrapper.js Outdated
Comment thread packages/formdata-event/ts_src/environment/element.ts Outdated
bicknellr
bicknellr reviewed Mar 24, 2023
View reviewed changes
Comment thread packages/shadydom/src/patches/Element.js
bicknellr
bicknellr reviewed Mar 24, 2023
View reviewed changes
Comment thread packages/shadydom/src/patches/Element.js Outdated
bicknellr
bicknellr reviewed Mar 24, 2023
View reviewed changes
Comment thread packages/shadydom/src/patches/Element.js
justinfagnani and others added 9 commits March 24, 2023 15:22
@justinfagnani @bicknellr
Update packages/custom-elements/ts_src/Patch/Element.ts
2be8ed0 
Co-authored-by: Russell Bicknell <bicknellr@google.com>
@justinfagnani @bicknellr
Update packages/shadydom/src/patches/Element.js
1603f68 
Co-authored-by: Russell Bicknell <bicknellr@google.com>
@justinfagnani @bicknellr
Update packages/shadydom/src/wrapper.js
a59e6d5 
Co-authored-by: Russell Bicknell <bicknellr@google.com>
@justinfagnani @bicknellr
Update packages/custom-elements/ts_src/Patch/Element.ts
467916e 
Co-authored-by: Russell Bicknell <bicknellr@google.com>
@justinfagnani
Add tests
064d3ba
@justinfagnani
Fix test
58762f6
@justinfagnani
Fix tests
3007766
@justinfagnani
format
b71d8b9
@justinfagnani
Update changelogs
94abb32
@justinfagnani

justinfagnani commented Mar 24, 2023

Copy link
Copy Markdown
Collaborator Author

Added some tests. I think this is mostly in the right shape now, except for actually adding a toggleAttribute() polyfill for IE. The tests at least cover toggleAttribute() patching behavior in browsers that have it natively.

@justinfagnani justinfagnani marked this pull request as ready for review March 25, 2023 00:09
@justinfagnani justinfagnani requested a review from aomarks as a code owner March 25, 2023 00:09
@justinfagnani justinfagnani requested a review from sorvell March 25, 2023 00:09
@justinfagnani justinfagnani changed the title WIP: add toggleAttribute Add Element.toggleAttribute() polyfill and support in Shady DOM Mar 25, 2023
@justinfagnani

justinfagnani commented Mar 25, 2023

Copy link
Copy Markdown
Collaborator Author

Ready for review!

sorvell
sorvell requested changes Mar 25, 2023
View reviewed changes
Comment thread packages/tests/webcomponentsjs_/toggle-attribute.html Outdated
Comment thread packages/shadydom/src/patches/Element.js
bicknellr
bicknellr requested changes Mar 25, 2023
View reviewed changes
Comment thread packages/custom-elements/CHANGELOG.md Outdated
Comment thread packages/formdata-event/ts_src/environment/element.ts Outdated
Comment thread packages/shadydom/CHANGELOG.md Outdated
Comment thread packages/shadydom/src/patches/Element.js Outdated
Comment thread packages/shadydom/src/patches/Element.js Outdated
Comment thread packages/tests/custom-elements/html/Element/toggleAttribute.test.html Outdated
Comment thread packages/tests/custom-elements/html/Element/toggleAttribute.test.html Outdated
Comment thread packages/tests/custom-elements/html/Element/toggleAttribute.test.html Outdated
Comment thread packages/tests/custom-elements/html/Element/toggleAttribute.test.html Outdated
Comment thread packages/custom-elements/ts_src/Patch/Element.ts Outdated
justinfagnani and others added 5 commits March 24, 2023 18:15
@justinfagnani @bicknellr
Update packages/custom-elements/CHANGELOG.md
d09644b 
Co-authored-by: Russell Bicknell <bicknellr@google.com>
@justinfagnani @bicknellr
Apply suggestions from code review
c3e25f7 
Co-authored-by: Russell Bicknell <bicknellr@google.com>
Co-authored-by: Steve Orvell <sorvell@google.com>
@justinfagnani
Address comments
c2173c8
@justinfagnani
Skip CE toggleAttribute tests if toggleAttribute not found
423274c
@justinfagnani
oops
02f4448
sorvell
sorvell reviewed Mar 27, 2023
View reviewed changes
Comment thread packages/shadydom/src/patches/Element.js
bicknellr
bicknellr requested changes Mar 27, 2023
View reviewed changes
Comment thread packages/formdata-event/ts_src/environment/element.ts Outdated
Comment thread packages/webcomponentsjs/ts_src/platform/toggle-attribute.ts
Comment thread packages/webcomponentsjs/ts_src/platform/toggle-attribute.ts Outdated
Comment thread packages/webcomponentsjs/ts_src/platform/toggle-attribute.ts Outdated
Comment thread packages/webcomponentsjs/ts_src/platform/toggle-attribute.ts Outdated
Comment thread packages/webcomponentsjs/ts_src/platform/toggle-attribute.ts Outdated
justinfagnani and others added 2 commits March 27, 2023 18:12
@justinfagnani
Address feedback
f5e6426
@justinfagnani @bicknellr
Apply suggestions from code review
def6a4e 
Co-authored-by: Russell Bicknell <bicknellr@google.com>
bicknellr
bicknellr approved these changes Mar 28, 2023
View reviewed changes

@bicknellr bicknellr left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I guess one thing we might want to test is the return values from toggleAttribute, otherwise LGTM for the custom elements polyfill and platform polyfills stuff.

Comment thread packages/tests/custom-elements/html/Element/toggleAttribute.test.html Outdated
@justinfagnani justinfagnani merged commit ee3a436 into master Mar 29, 2023
@justinfagnani justinfagnani deleted the toggle-attribute branch March 29, 2023 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bicknellr bicknellr bicknellr approved these changes
@aomarks aomarks Awaiting requested review from aomarks aomarks is a code owner
@sorvell sorvell Awaiting requested review from sorvell

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@justinfagnani @sorvell @bicknellr