Skip to content

Conversation

@userquin
Copy link
Contributor

@userquin userquin commented Feb 9, 2026

Using div inside a button is invalid html markup (hydration missmatch issues).

I've been reviewing the button usages using div inside:

  • @zeucapua review AccountMenu.client.vue
  • @serhalp review PackageSelector.vue and VulnerabilityTree.vue
  • @Flo0806 review DeprecatedTree.vue

@vercel
Copy link

vercel bot commented Feb 9, 2026

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

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 9, 2026 1:27pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 9, 2026 1:27pm
npmx-lunaria Ignored Ignored Feb 9, 2026 1:27pm

Request Review

@danielroe
Copy link
Member

danielroe commented Feb 9, 2026

thank you!! 🫶

I'm working on hydration right now, and this is so helpful.

#1248

@codecov
Copy link

codecov bot commented Feb 9, 2026

Codecov Report

❌ Patch coverage is 81.81818% with 4 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/components/Compare/PackageSelector.vue 60.00% 1 Missing and 1 partial ⚠️
app/components/Package/DeprecatedTree.vue 50.00% 1 Missing ⚠️
app/components/Package/VulnerabilityTree.vue 66.66% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 9, 2026

📝 Walkthrough

Walkthrough

This pull request changes block-level div wrappers to inline/inline-block span wrappers in four Vue components: app/components/Compare/PackageSelector.vue, app/components/Header/AccountMenu.client.vue, app/components/Package/DeprecatedTree.vue, and app/components/Package/VulnerabilityTree.vue. The edits adjust template element types and closing tags to alter DOM semantics and layout while preserving classes, conditional rendering, event handling, and data flow. No exported/public API signatures or component logic were modified.

Possibly related PRs

Suggested labels

front

Suggested reviewers

  • serhalp
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description directly relates to the changeset, explaining the specific issue (invalid HTML markup with divs inside buttons) and listing the affected files with assigned reviewers.

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

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
app/components/Package/DeprecatedTree.vue (1)

56-61: HTML fix looks correct; consider removing redundant block class.

The conversion from div to span correctly resolves the invalid HTML markup inside the button. However, the block class is redundant when flex is already present—display: flex inherently creates a block-level container, so block has no effect.

🧹 Optional cleanup
-        <span class="flex items-center gap-2 min-w-0 block">
+        <span class="flex items-center gap-2 min-w-0">

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

Copy link
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: 2

@userquin
Copy link
Contributor Author

userquin commented Feb 9, 2026

@danielroe can you apply coderabbitai suggestions? we may also need to add flex flex-col to some span at AccountMenu.client.vue

@danielroe danielroe mentioned this pull request Feb 9, 2026
2 tasks
@danielroe danielroe added this pull request to the merge queue Feb 9, 2026
Merged via the queue into npmx-dev:main with commit b21b58c Feb 9, 2026
17 checks passed
@userquin userquin deleted the fix-invalid-html-button-markup branch February 9, 2026 14:51
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.

2 participants