Skip to content

Conversation

@babielmam
Copy link

This allows to use the data-inert attribute to allow the polyfill and have valid HTML syntax. Related to #126

@babielmam
Copy link
Author

@robdodson @alice please have a look at the PR I created. I also added some test, that I copied from the basic tests.
I added a linebreak-style option to the ES-lint file because my IDE was otherwise reporting them as false. I think it shouldn't hurt to have it in there.

@robdodson
Copy link
Collaborator

Hey @babielmam, quick question: does this actually set data-inert on any of the elements?

@babielmam
Copy link
Author

babielmam commented Sep 5, 2019

No, it doesn't set the data-inert attribute.
As I looked over the code just right now, I see some cases where only inert is watched for but not data-inert. So, if you wish I can rework those parts

@alice
Copy link
Member

alice commented Sep 6, 2019

Sorry for being so slow to get to this.

The code mostly looks good, but I'd like a couple more tests as suggested in my comment.

@babielmam
Copy link
Author

babielmam commented Oct 4, 2019

@alice , @robdodson I reworked the parts mentioned in my last comment. I also added some more tests. Please take a look :)

@robdodson
Copy link
Collaborator

I think Alice is out of office till the 13th, but I'll see if she and I can catch up on this then.

@bkardell
Copy link
Collaborator

Just looking at it, it looks pretty good I, think. It seems just a little unfortunate to me that this has to be a thing, but it seems like a reasonable request and actual contribution.

Copy link
Member

@alice alice left a comment

Choose a reason for hiding this comment

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

I don't think it's necessary to set data-inert, since folks who prefer to use the data attribute can use el.dataset.inert.

@@ -0,0 +1,29 @@
describe('Data-Attribute', function() {
Copy link
Member

Choose a reason for hiding this comment

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

Could you also add some tests for mutations?

  • Adding an element into an inert subtree
  • Adding data-inert to an existing element
  • Adding an element to the DOM with a data-inert attribute already on it

@yoavweiss yoavweiss changed the base branch from master to main March 5, 2021 21: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.

5 participants