fix: close the review followups from PR #166 - #185
Closed
diegokingston wants to merge 1 commit into
Closed
Conversation
- Remove MobileResultsPanel entirely: after its opener button was removed, the panel could only appear on top of the results sheet after a solve and never be reopened. The sheet is the results surface now. Drops the component, its store field, the auto-open writes in solve.ts/live-calc.ts, its locale keys in all 12 locales, and the zup-field-names assertion that read the file. - Remove dead leftDrawerOpen writes (the left drawer renders nowhere) and tour-steps.ts, which had no importers left. - SheetGrab: tear down the window-level drag listeners on destroy, guard against a second pointer mid-drag, drop the callerless onResizeEnd. - Re-frame the canvas when the PRO sheet opens/closes too, not just Basic's — the same watcher covers both. - Educacion's right drawer takes --st-sheet-h instead of hardcoded 58vh. - Tour: the sections/materials steps open the data sheet in their own onEnter instead of relying on earlier steps leaving it open. - Rewrite stale App.svelte comments describing removed controls; drop dead .pmt-btn.active / .rb-cmd.prominent CSS and the unused ribbon.groupData locale key.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes for the review findings posted in #166 (comment). Base is
basic/mobile-ui(the #166 head), notmain.What changed
Should-fix
MobileResultsPanelremoved entirely. After its opener button was deleted, the panel could only auto-appear on top of the results sheet after a solve and never be reopened. The sheet is the results surface now, so the panel goes: component, mount, store field (mobileResultsPanelOpen), the auto-open writes insolve.ts/live-calc.ts, its locale keys in all 12 locales, and thezup-field-namesassertion that read the component file.leftDrawerOpenwrites removed fromsolve.ts(and the mode-switch handler inApp.svelte);tour-steps.tsdeleted — it had no importers left.App.svelterewritten to describe what the markup actually does now (.pmt-wrap,.pmt-btn).Nits
SheetGrab:onDestroyteardown for the window-level drag listeners, a one-drag-at-a-time guard (multitouch), and the callerlessonResizeEndprop removed.sheetWasOpenwatcher, keyed onrightDrawerOpenfor PRO), not just Basic..drawer-rightusesvar(--st-sheet-h)instead of hardcoded58vh..pmt-btn.active,.rb-cmd.prominent; unusedribbon.groupDatalocale key dropped.sections/materialssteps open the data sheet in their ownonEnterrather than relying on an earlier step leaving it open.Not addressed:
audit-demos.mjsstill audits walkthroughs only at 1500×950 (dev script, not CI) — happy to add the viewport argument here if wanted.Verification
npm run typecheck: 479 = baseline, no new errors.npm run test: unit + build passes green (includes the updatedzup-field-names).npx playwright test: not run locally; CI e2e covers it.