Skip to content

fix: fix html-validation error#1658

Draft
eryue0220 wants to merge 6 commits intonpmx-dev:mainfrom
eryue0220:fix/fix-html-validation-error
Draft

fix: fix html-validation error#1658
eryue0220 wants to merge 6 commits intonpmx-dev:mainfrom
eryue0220:fix/fix-html-validation-error

Conversation

@eryue0220
Copy link
Copy Markdown
Contributor

@eryue0220 eryue0220 commented Feb 26, 2026

🔗 Linked issue

Resolve #1425

🧭 Context

Fix html-validation lint error when visit page /package/@pnpm/workspace.find-packages

📚 Description

  • Fix the aria id if the dropdown is not open
  • Disable the prefer-native-element when the element role is listbox

@vercel
Copy link
Copy Markdown

vercel bot commented Feb 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs.npmx.dev Ready Ready Preview, Comment Apr 17, 2026 10:50am
npmx.dev Ready Ready Preview, Comment Apr 17, 2026 10:50am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
npmx-lunaria Ignored Ignored Apr 17, 2026 10:50am

Request Review

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 26, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Trigger buttons in two dropdown components now set aria-controls only when open. Both dropdown containers render persistently using v-show with :aria-hidden to reflect visibility. aria-activedescendant computation and active-item id comparisons were tightened. Nuxt HTML validator rules added a prefer-native-element exception for listbox.

Changes

Cohort / File(s) Summary
Dropdown: Package manager
app/components/Package/ManagerSelect.vue
Button aria-controls set to isOpen ? listboxId : undefined; dropdown switched from v-if to v-show and now includes :aria-hidden="!isOpen".
Dropdown: README TOC
app/components/ReadmeTocDropdown.vue
Button aria-controls conditional on isOpen; dropdown uses v-show + :aria-hidden; aria-activedescendant only computed when highlighted index and id exist; active-id comparisons normalized.
Config: HTML validation
nuxt.config.ts
Added HTML validator rule: prefer-native-element with { exclude: ['listbox'] } alongside existing meta-refresh rule.
Misc / manifest
manifest_file, package.json
Metadata/package files touched (minor lines changed).

Possibly related PRs

Suggested reviewers

  • danielroe
  • whitep4nth3r
🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive The PR partially addresses issue #1425's objective to resolve missing-id reference errors, but current errors remain for v-0-3-listbox and v-0-6-toc-listbox IDs despite the fixes applied. Verify whether the changes fully resolve all missing-id validation errors reported in #1425, or if additional adjustments are needed to eliminate remaining errors.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change as fixing HTML-validation errors, which aligns with the primary objective of resolving validation issues.
Description check ✅ Passed The description is directly related to the changeset, explaining the specific fixes applied: adjusting aria id conditionally and disabling prefer-native-element rule for listbox elements.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing HTML validation errors: conditional aria-controls binding, visibility toggling with aria-hidden, tightened aria-activedescendant logic, and htmlValidator configuration.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e61657e and 65fcacd.

📒 Files selected for processing (3)
  • app/components/Package/ManagerSelect.vue
  • app/components/ReadmeTocDropdown.vue
  • nuxt.config.ts

Comment thread app/components/ReadmeTocDropdown.vue Outdated
Comment thread nuxt.config.ts
rules: { 'meta-refresh': 'off' },
rules: {
'meta-refresh': 'off',
'prefer-native-element': ['error', { exclude: ['listbox'] }],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤔 Per https://discord.com/channels/1464542801676206113/1464926468751753269/1471511992434163844, I think if anything we'd want exactly the opposite (include), wouldn't we? As is this would suppress warnings about role=listbox, which we do want to eliminate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So the goal is to rewrite those components which their attribute is role=listbox, right?

Copy link
Copy Markdown
Contributor

@ghostdevv ghostdevv left a comment

Choose a reason for hiding this comment

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

@eryue0220 would you be up for updating this PR? I think it is still valid, not sure if these are the same issues though 🙏

 ERROR  HTML validation errors found for /package/@pnpm/workspace.find-packages                                                                                                    23:03:55

inline
  38:34256  error  Element references missing id "v-0-3-listbox"      no-missing-references
  38:77153  error  Element references missing id "v-0-6-toc-listbox"  no-missing-references

✖ 2 problems (2 errors, 0 warnings)

@eryue0220
Copy link
Copy Markdown
Contributor Author

@eryue0220 would you be up for updating this PR? I think it is still valid, not sure if these are the same issues though 🙏

 ERROR  HTML validation errors found for /package/@pnpm/workspace.find-packages                                                                                                    23:03:55

inline
  38:34256  error  Element references missing id "v-0-3-listbox"      no-missing-references
  38:77153  error  Element references missing id "v-0-6-toc-listbox"  no-missing-references

✖ 2 problems (2 errors, 0 warnings)

Okay, I'll check that and update these cases.

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.

HTML validation errors

3 participants