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

Fixes #3424, conflicts with existing id attributes from IdAttributePlugin #3430

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

zachleat
Copy link
Member

@zachleat zachleat commented Sep 3, 2024

Automatically resolve conflicts with other elements on the page that have an id attribute.

Conflict resolution example 1

<div id="testing"></div>
<h1>Testing</h1>

becomes

<div id="testing"></div>
<h1 id="testing-2">Testing</h1>

Conflict resolution example 2

<h1>Testing</h1>
<div id="testing"></div>

becomes

<h1 id="testing-2">Testing</h1>
<div id="testing"></div>

Filter callback

Also adds a filter callback option to IdAttributePlugin per requested at #3424.

eleventyConfig.addPlugin(IdAttributePlugin, {
	filter: function({ page }) {
		if(page.inputPath.endsWith("test-skipped.html")) {
			return false;
		}
		return true;
	}
});

@zachleat zachleat added this to the Eleventy 3.0.0 milestone Sep 3, 2024
@zachleat zachleat merged commit d8bd086 into main Sep 3, 2024
20 checks passed
@zachleat zachleat deleted the issue-3424 branch September 3, 2024 17:04
@zachleat
Copy link
Member Author

zachleat commented Sep 3, 2024

Ships with 3.0.0-beta.2 and 3.0.0-alpha.19

@vrugtehagel
Copy link
Contributor

Note; with this solution it is still unsafe to add the plugin to an existing codebase, since it can rename elements that have an specific id (and might be relied upon by styles or scripts) if they come after a header that receives the same id.

@zachleat
Copy link
Member Author

zachleat commented Sep 4, 2024

Yeah, tried to communicate that with the warning above.

To workaround that limitation would require a two-pass algorithm to find all existing id attributes up front. Might it be better to throw an error rather than rename any existing id?

@zachleat
Copy link
Member Author

zachleat commented Sep 4, 2024

I wonder if a better option is to allow a prefix option to add to any assigned heading ids to further mitigate conflicts

@zachleat
Copy link
Member Author

zachleat commented Sep 4, 2024

Ah, nevermind—I found an approach that avoids two-pass and doesn’t rename existing ids!

zachleat added a commit that referenced this pull request Sep 4, 2024
@zachleat
Copy link
Member Author

zachleat commented Sep 4, 2024

I updated the examples in the first comment to reflect the new logic. Existing id attributes in your markup are not modified.

@vrugtehagel
Copy link
Contributor

Thanks a bunch!

zachleat added a commit to 11ty/11ty-website that referenced this pull request Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants