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

Audit: new dom nodes table is confusing #5220

Closed
ebidel opened this issue May 15, 2018 · 6 comments · Fixed by #6065
Closed

Audit: new dom nodes table is confusing #5220

ebidel opened this issue May 15, 2018 · 6 comments · Fixed by #6065
Assignees

Comments

@ebidel
Copy link
Contributor

ebidel commented May 15, 2018

I'm not sure what the last line is telling me. It's identified two nodes, but they're not explained or lined up to anything.

screen shot 2018-05-15 at 6 41 07 pm

@paulirish
Copy link
Member

cc @hwikyounglee for ideas

@exterkamp
Copy link
Member

I started to experiment on this a bit here.

The table now has three columns: Category, Element, and Count. I also parse down the element snippet to be only the HTML element type + the id or class and drop every other attribute so that it doesn't get too large.

screen shot 2018-09-17 at 5 49 36 pm

Thoughts?

@exterkamp exterkamp self-assigned this Sep 18, 2018
@exterkamp
Copy link
Member

Notes from meeting:

@paulirish how do we want to trim HTML elements? Should it be additive or subtractive? I vote additive so we can only include what is necessary instead of trying to remove as much as possible. Right now the only things I keep are:

  • id="..." || class="..."
  • I know you mentioned keeping aria-*="..." elements as well

@paulirish
Copy link
Member

@exterkamps ive only seen [style] be a problem. my concern is that we'll strip some attributes that are useful. [data-*] come to mind.
but there could be others.

if we are subtractive then there's a much smaller risk that we effed up and removed important data.
and i presume it's more straightforward see that we need to remove another attribute than for people to tell us that we've excluded an attribute they need to identify the element. 🤷‍♂️

@exterkamp
Copy link
Member

That's fair, and easier to implement. I'll strip out just style=".*" tags then.

And did we want to make the stripping of snippets more generic, or put it into some lib/ file so that all snippets can use it? i.e. the accessibility snippets

@paulirish
Copy link
Member

And did we want to make the stripping of snippets more generic, or put it into some lib/ file so that all snippets can use it? i.e. the accessibility snippets

yah good call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants