Skip to content

Conversation

coliff
Copy link
Member

@coliff coliff commented May 27, 2025

Fixes: #1297

Add rel to list of sorted attributes

@coliff coliff requested a review from thedaviddias as a code owner May 27, 2025 04:31
@github-actions github-actions bot added the test label May 27, 2025
gemini-code-assist[bot]

This comment was marked as resolved.

Copy link

codecov bot commented May 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.46%. Comparing base (95e6a13) to head (528daa8).
Report is 12 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1579      +/-   ##
==========================================
+ Coverage   96.99%   98.46%   +1.47%     
==========================================
  Files           2        2              
  Lines        1628     1629       +1     
  Branches      335      342       +7     
==========================================
+ Hits         1579     1604      +25     
+ Misses         49       25      -24     
Files with missing lines Coverage Δ
dist/htmlhint.js 98.46% <100.00%> (+1.47%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0494c1e...528daa8. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request clearly and effectively adds the rel attribute to the list of sorted attributes for the attr-sorted rule. The changes are well-implemented in the source code, and the accompanying tests provide good coverage for the new functionality, especially the interaction between rel and href.

Well done on:

  • Correctly updating the sortOrder array in src/core/rules/attr-sorted.ts.
  • Adding specific and relevant test cases in test/rules/attr-sorted.spec.js to verify the new behavior.
  • Ensuring the generated distribution files (dist/) are updated accordingly.

The placement of rel (after type and before href) in the sort order seems logical and aligns with common usage patterns, for example, in <link> tags.

One minor suggestion for follow-up:

  • Please ensure the documentation for the attr-sorted rule (likely in website/src/content/docs/rules/attr-sorted.md) is updated to include rel in the specified attribute order. This will help users understand the new correct ordering.

Overall, this is a solid and straightforward enhancement.

Summary of Findings

  • Code Quality and Correctness: The changes are correctly implemented, and the logic for sorting attributes remains sound with the addition of rel.
  • Testing: New test cases effectively validate the behavior of the rel attribute in relation to href.
  • Documentation (Out of Scope of Diff): A recommendation was made to update the rule's documentation to reflect the new sort order including the rel attribute.

Merge Readiness

The pull request is in good shape and the changes are well-implemented. I recommend merging these changes after considering the documentation update for the attr-sorted rule. As an AI reviewer, I am not authorized to approve pull requests; please ensure a human reviewer also approves this change before merging.

@coliff coliff added the rule Relates to HTMLHint's core rules label May 27, 2025
@coliff coliff merged commit a38ddef into main May 27, 2025
17 checks passed
@coliff coliff deleted the dev/coliff/attr-sorted-rel branch May 27, 2025 04:35
@coliff coliff mentioned this pull request May 30, 2025
@coliff coliff changed the title Add rel to list of sorted attributes feat(attr-sorted): Add rel to list of sorted attributes May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Relates to HTMLHint's core rules test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add rel to list of sorted attributes
1 participant