Skip to content
This repository was archived by the owner on Mar 20, 2026. It is now read-only.

EditCounter: + logscale option for monthyearcharts - #507

Merged
MusikAnimal merged 15 commits into
x-tools:mainfrom
agoujot:logscale
May 16, 2025
Merged

MusikAnimal merged 15 commits into
x-tools:mainfrom
agoujot:logscale

Conversation

@agoujot

@agoujot agoujot commented Apr 24, 2025

Copy link
Copy Markdown
Member

Has been requested a few times.

Implementation details:

  • linked each checkbox to both charts, because it's smoother. so like if you change one it also changes the other checkbox and both charts change scale.
  • I added for this the message use-log-scale in qqq.json, en.json and fr.json. Do tell me if there's something else I need to do to setup i18n.
  • has to start at 1, else we get a log(0) infinite loop
  • had to enforce an at least 1.5 coefficient between two ticks in xAxis else we'd get a lot of overlapping
  • and, and this is the part which is a bit meh, the way it works is tearing out the chart and recreating it with different options. I spent an hour or two trying to update the config dynamically, but it failed miserably every time. I don't think there's a way we can do that in chartjs 2.7. (or it isn't documented anymore... who knows.)

Bug: T177658

@MusikAnimal MusikAnimal left a comment

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.

This causes my browser to crash. I used enwiki user GoldenRing as an example. That user has a long tenure, but relatively few edits. I tried on a few other accounts and had the same problem.

We appear to be using Chart.js version 2.3.0 in Pageviews Analysis, and the toggle for the logarithmic scale there seems to work fine. I don't know if any inspiration can be drawn from it (it's admittedly old and convoluted code), but here's where it happens:

It does appear, however, that I also had to destroy the chart and rebuild. That I suppose is fine; you don't even notice really, at least on my machine.

@codecov

codecov Bot commented May 14, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.99%. Comparing base (4b8eb28) to head (e86bd67).
Report is 87 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #507      +/-   ##
============================================
- Coverage     71.30%   70.99%   -0.32%     
- Complexity     1275     1298      +23     
============================================
  Files            46       46              
  Lines          3792     3913     +121     
============================================
+ Hits           2704     2778      +74     
- Misses         1088     1135      +47     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@agoujot agoujot closed this May 14, 2025
@agoujot agoujot reopened this May 14, 2025
@agoujot
agoujot requested a review from MusikAnimal May 14, 2025 12:23
@agoujot

agoujot commented May 14, 2025

Copy link
Copy Markdown
Member Author

Huurrrggghhh. After a lot of head-banging and thinking I found it three times, I finally found it. It's one of the stupidest bugs I ever saw. Until ChartJS PR 4959, which was in 2.7.2 (we're in 2.7.0 🥲), every time a log scale is given a zero, it goes infinite-loopy.

Well, anyhow. In the end the fix is min:1, in log scale.

Heh, what was I saying the other day about the amount of work for six bytes? This time about 5 hours. Don't mind the close-reopen, just a misclick.

@MusikAnimal MusikAnimal left a comment

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.

Logarithmic scales can't begin at zero anyway, and there are no fractional numbers here, so starting at 1 is fine. Pageviews Analysis appears to do the same thing.

Thank you for your hard work!! :D

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants