Skip to content

fix(ui): remove redundant calendar icons from date inputs#2042

Merged
alexdln merged 3 commits intonpmx-dev:mainfrom
marlonwq:fix/remove-extra-calendar-icon
Mar 12, 2026
Merged

fix(ui): remove redundant calendar icons from date inputs#2042
alexdln merged 3 commits intonpmx-dev:mainfrom
marlonwq:fix/remove-extra-calendar-icon

Conversation

@marlonwq
Copy link
Contributor

🧭 Context

Resolves #2038

📚 Description

This PR removes the manual calendar icons, preventing duplication.

🛠️ Changes

  • Removed redundant <span> icons with class i-lucide:calendar.
  • Adjusted InputBase padding from ps-7 to ps-2 to align text correctly after icon removal.

🧪 Testing

  • Verified on Chrome (Desktop): Only the native icon remains, picker works as expected.
image image

@vercel
Copy link

vercel bot commented Mar 12, 2026

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

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Mar 12, 2026 8:34am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Mar 12, 2026 8:34am
npmx-lunaria Ignored Ignored Mar 12, 2026 8:34am

Request Review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 12, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 74ac2bab-1283-4719-a687-fffc5db447c1

📥 Commits

Reviewing files that changed from the base of the PR and between 63f97d9 and 88be4a4.

📒 Files selected for processing (1)
  • app/components/Package/TrendsChart.vue
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/components/Package/TrendsChart.vue

📝 Walkthrough

Walkthrough

Removed the calendar icon span elements that preceded the startDate and endDate inputs in TrendsChart.vue. Removed the ps-7 right-padding class from both date input elements, reducing horizontal spacing. No exported or public declarations were changed.

Possibly related PRs

Suggested reviewers

  • alexdln
  • graphieros
  • danielroe
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The PR description clearly relates to the changeset, describing removal of calendar icons and padding adjustment from TrendsChart.vue.
Linked Issues check ✅ Passed The PR successfully addresses issue #2038 by removing redundant calendar icon spans and adjusting padding, meeting all stated coding requirements.
Out of Scope Changes check ✅ Passed All changes in TrendsChart.vue are directly scoped to removing manual calendar icons and adjusting padding, with no unrelated modifications.

✏️ 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

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.

@marlonwq marlonwq changed the title ui: remove redundant calendar icon from date inputs fix(ui): remove redundant calendar icons from date inputs Mar 12, 2026
@codecov
Copy link

codecov bot commented Mar 12, 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!

Copy link
Contributor

@graphieros graphieros left a comment

Choose a reason for hiding this comment

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

Thank you :)

@graphieros graphieros added this pull request to the merge queue Mar 12, 2026
@alexdln alexdln removed this pull request from the merge queue due to a manual request Mar 12, 2026
Copy link
Member

@shuuji3 shuuji3 left a comment

Choose a reason for hiding this comment

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

LGTM👍🏻 I also tested on Firefox on desktop and Android.

The mobile view still hide the day, but I think this is a separate issue 😅

screenshot of download chart dialog. right day digit is partically hidden with calendar icon

@shuuji3 shuuji3 added this pull request to the merge queue Mar 12, 2026
@shuuji3 shuuji3 removed this pull request from the merge queue due to a manual request Mar 12, 2026
@alexdln
Copy link
Member

alexdln commented Mar 12, 2026

@shuuji3 sorry, minor changes request 🙃

@shuuji3
Copy link
Member

shuuji3 commented Mar 12, 2026

No problem! I thought I canceled the queue by approving it 😄

@graphieros
Copy link
Contributor

The mobile view still hide the day, but I think this is a separate issue 😅

FIxed in https://github.com/npmx-dev/npmx.dev/pull/2037/changes

marlonwq and others added 2 commits March 12, 2026 05:32
Co-authored-by: Alex Savelyev <91429106+alexdln@users.noreply.github.com>
Co-authored-by: Alex Savelyev <91429106+alexdln@users.noreply.github.com>
@alexdln alexdln enabled auto-merge March 12, 2026 08:33
@marlonwq
Copy link
Contributor Author

Wow, thanks for the quick reply!

I've implemented the suggestions 🙂

@graphieros
Copy link
Contributor

I'm glad this one is in :)

Merged via the queue into npmx-dev:main with commit e0a31c3 Mar 12, 2026
20 checks passed
@github-actions github-actions bot mentioned this pull request Mar 12, 2026
@marlonwq marlonwq deleted the fix/remove-extra-calendar-icon branch March 12, 2026 08:46
@lukewarlow
Copy link
Contributor

@graphieros that's not correct btw, firefox android does still have a bug but it seems related to the w-full and min-w-0 on the input. Chrome android had a separate bug that has been fixed by that patch.

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.

Consider removing additional calendar icon from date inputs

5 participants