Skip to content

Comments

fix: increase chart right padding#1620

Open
graphieros wants to merge 1 commit intomainfrom
1618-in-some-languages-the-weeklydownload-modal-metric-values-are-not-displayed-completely
Open

fix: increase chart right padding#1620
graphieros wants to merge 1 commit intomainfrom
1618-in-some-languages-the-weeklydownload-modal-metric-values-are-not-displayed-completely

Conversation

@graphieros
Copy link
Contributor

@graphieros graphieros commented Feb 24, 2026

Resolves #1618

  • Increases the right padding value of the chart to fit magnitude labels in some languages
    (tested with 999.9)

@vercel
Copy link

vercel bot commented Feb 24, 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 24, 2026 6:23am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Feb 24, 2026 6:23am
npmx-lunaria Ignored Ignored Feb 24, 2026 6:23am

Request Review

@codecov
Copy link

codecov bot commented Feb 24, 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
Contributor

coderabbitai bot commented Feb 24, 2026

📝 Walkthrough

Walkthrough

The chart configuration in the TrendsChart component has been modified to increase the right padding from 100 to 128 pixels. This adjustment provides additional space for the final datapoint labels to render without truncation. The change affects a single line in the manifest file and does not alter any exported or public signatures.

Suggested reviewers

  • ghostdevv
  • danielroe
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR addresses issue #1618 by increasing chart right padding to ensure metric values render fully in the WeeklyDownload modal across locales.
Out of Scope Changes check ✅ Passed The change is narrowly scoped to adjusting chart right padding in TrendsChart.vue, directly addressing the truncation issue reported in #1618.
Description check ✅ Passed The pull request description clearly relates to the changeset, referencing issue #1618 and explaining the padding increase to accommodate magnitude labels across different languages.

✏️ 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
  • Commit unit tests in branch 1618-in-some-languages-the-weeklydownload-modal-metric-values-are-not-displayed-completely

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
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.

🧹 Nitpick comments (1)
app/components/Package/TrendsChart.vue (1)

1413-1413: LGTM — padding increase directly addresses locale-specific label truncation.

The bump from 100 → 128 gives the injected SVG <text> labels in drawLastDatapointLabel 28 additional pixels of breathing room, which resolves the reported truncation for Cyrillic and CJK compact-number strings (e.g., "30,4 млн.").

One optional consideration: drawLastDatapointLabel uses a hardcoded font-size="24" that does not scale with isMobile.value, whereas every other text size in the chart config doubles on mobile (isMobile.value ? 24 : 16, isMobile.value ? 32 : 24). If that font size is ever made responsive, the right padding would need revisiting. Given the current code keeps font-size fixed at 24, the static value of 128 is internally consistent.

🔍 If drawLastDatapointLabel is ever made mobile-aware, align the padding accordingly
-      padding: { bottom: displayedGranularity.value === 'yearly' ? 84 : 64, right: 128 }, // padding right is set to leave space of last datapoint label(s)
+      padding: { bottom: displayedGranularity.value === 'yearly' ? 84 : 64, right: isMobile.value ? 192 : 128 }, // padding right is set to leave space of last datapoint label(s)

This would only be relevant once font-size in drawLastDatapointLabel is also made responsive.


ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f4943b9 and 6d7503d.

📒 Files selected for processing (1)
  • app/components/Package/TrendsChart.vue

@graphieros graphieros enabled auto-merge February 24, 2026 06:27
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.

In some languages, the WeeklyDownload modal metric values are not displayed completely.

1 participant