Skip to content

fix: fetch last 10 average when enabling showAverage (@d1rshan)#7963

Open
d1rshan wants to merge 4 commits into
monkeytypegame:masterfrom
d1rshan:fix/modes-notice-update
Open

fix: fetch last 10 average when enabling showAverage (@d1rshan)#7963
d1rshan wants to merge 4 commits into
monkeytypegame:masterfrom
d1rshan:fix/modes-notice-update

Conversation

@d1rshan
Copy link
Copy Markdown
Contributor

@d1rshan d1rshan commented May 17, 2026

Description

Modes notice for last 10 average was not updating when changing showAverage from "off" to a different value after page reload because Last10Average.update() never ran (it's only called during test restart) and data stayed at default values (wpm = 0, acc = 0) and hence button was hidden.

Fix

Fixed by adding a configEvent listener in modes-notice.ts that fetches the average data before updating the UI when showAverage is enabled.

Closes #7961

Copilot AI review requested due to automatic review settings May 17, 2026 12:31
@monkeytypegeorge monkeytypegeorge added the frontend User interface or web stuff label May 17, 2026
@github-actions github-actions Bot added the waiting for review Pull requests that require a review before continuing label May 17, 2026
Comment thread frontend/src/ts/elements/modes-notice.ts Outdated
@github-actions github-actions Bot removed the waiting for review Pull requests that require a review before continuing label May 17, 2026
@d1rshan d1rshan force-pushed the fix/modes-notice-update branch from 2231f72 to a082dee Compare May 17, 2026 13:14
Copy link
Copy Markdown
Contributor

@Leonabcd123 Leonabcd123 left a comment

Choose a reason for hiding this comment

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

Not sure how you tested this, but it's still broken for me when following the reproduction steps in the bug report. There seems to be a race condition going on between the update function in last-10-average and update in modes-notice. This:

const avgWPM = Last10Average.getWPM();
const avgAcc = Last10Average.getAcc();

Runs before this:

averageWPM = Config.alwaysShowDecimalPlaces ? wpm : Math.round(wpm);
averageAcc = Config.alwaysShowDecimalPlaces ? acc : Math.floor(acc);

@d1rshan
Copy link
Copy Markdown
Contributor Author

d1rshan commented May 17, 2026

hey @Leonabcd123 yeah ik i thought it worked because i left a stale line in modes-notice.ts to update everytime 😭 (that is what i force pushed ) i realized about the race condition, i'm figuring out what is the best solution here.

we could do this, but will trigger ModesNotice.update() twice, so not sure if this is a good idea.

configEvent.subscribe(({ key, newValue }) => {
if (key === "showAverage" && newValue !== "off") {
void update().then(() => ModesNotice.update());
}
});

@Miodec Miodec marked this pull request as draft May 19, 2026 15:06
@d1rshan d1rshan marked this pull request as ready for review May 19, 2026 15:49
@github-actions github-actions Bot added the waiting for review Pull requests that require a review before continuing label May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend User interface or web stuff waiting for review Pull requests that require a review before continuing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Modes notice for last10 average not updating

4 participants