feat(label-generator): print button, maker layout, and print rotation#1536
Open
Wrr2216 wants to merge 3 commits into
Open
feat(label-generator): print button, maker layout, and print rotation#1536Wrr2216 wants to merge 3 commits into
Wrr2216 wants to merge 3 commits into
Conversation
Move pure label-generation logic into frontend/lib/reports/label-generator.ts (types and functions: fmtAssetID, calculateGridData, calculateMakerGrid, makerPageSize, buildPageCss, presetFor, normalizeMeasure) and add comprehensive unit tests. Update the label-generator Vue page to import and use the new module, add an output-mode selector (sheet / label maker / custom), maker-specific inputs (labelsPerRow, labelGap), seed presets for modes, compute maker page size and inject @page CSS, and adapt page/grid calculation flow to handle maker vs sheet. Also add new localization strings for the label maker UI.
A handful of fixes to the label generator, mostly to get the label maker (continuous tape) output printing the way it should. - Add a Print button next to Generate Page that regenerates the labels and opens the print dialog. The form is already hidden in print, so only the labels come out. - Lay the maker labels out across the page and wrap them instead of stacking one per row, and force one label per printed page so each lands on its own tape segment. - Default the maker preset to a Brother QL DK-2205 roll (2.4" wide, 1" tall), which is what these are actually sized for. - Add a Print rotation option (None/90/180/270) for printers that rotate the page onto the tape, plus a note pointing people at the system print dialog where the orientation actually lives. - Key the property inputs by ref instead of index. Switching modes was recycling a number input into the base URL field, which threw a "value cannot be parsed" error and scrambled the inputs. - Add unit tests for the page and rotation CSS helpers.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
Summary by CodeRabbit
WalkthroughExtracts label geometry, grid calculations, and print CSS into a reusable TypeScript library and integrates it into the label-generator Vue page to add sheet/maker/custom modes, maker-specific inputs, print rotation, tests, and updated English locale keys. ChangesLabel Generator Library and Mode Support
Sequence Diagram(s)sequenceDiagram
participant User
participant Component as label-generator.vue
participant GridLib as label-generator lib
participant CSS as buildPageCss/buildRotateCss
participant Print as Browser Print
User->>Component: Select mode / adjust inputs
Component->>GridLib: presetFor(mode)
GridLib-->>Component: LabelPreset
Component->>Component: Apply preset & update displayProperties
User->>Component: Generate page
Component->>GridLib: calculateGridData() or calculateMakerGrid()
GridLib-->>Component: GridData or error
Component->>CSS: buildPageCss(mode, rotation)
CSS-->>Component: `@page` rule (maker only)
Component->>CSS: buildRotateCss(mode, rotation)
CSS-->>Component: rotation CSS
Component->>Print: printLabels() -> window.print()
Print-->>User: Browser print dialog
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🔒 Security Recommendations
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
frontend/lib/reports/label-generator.test.ts (1)
176-181: ⚡ Quick winAdd a 270° rotation assertion to lock symmetry behavior.
buildRotateCsstests currently skip 270°, leaving one branch unguarded. Add a dedicated 270 case so rotation recentering regressions are caught.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/lib/reports/label-generator.test.ts` around lines 176 - 181, Add a new test that asserts buildRotateCss("maker", size, 270) produces the symmetric recentering CSS (mirror of the 90° case) so the 270° rotation branch is covered; locate the test block in label-generator.test.ts near the existing 90° test and add a case named like "270 rotation sizes and re-centers the label onto the swapped page" that calls buildRotateCss with rotation 270 and expects the appropriate `@media` print CSS string for 270deg with the corresponding translate offsets and transform-origin.frontend/pages/reports/label-generator.vue (1)
403-409: ⚡ Quick winHarden dynamic print CSS inputs before injecting style
innerHTML.At Line 407–408, generated CSS is injected via
innerHTML. Please clamp/normalize numeric dimensions (labelWidth,labelHeight,labelsPerRow,labelGap) to finite bounds before CSS generation to reduce malformed CSS injection surface from tampered client state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/pages/reports/label-generator.vue` around lines 403 - 409, The generated CSS is injected via useHead innerHTML using buildPageCss and buildRotateCss with values from mode.value, makerSize.value and printRotation.value; before calling those functions, validate and clamp any numeric inputs (e.g., labelWidth, labelHeight, labelsPerRow, labelGap, rotation) taken from makerSize or printRotation to finite, reasonable bounds (use Number.isFinite checks and min/max limits) and normalize types (parseFloat/Int) so buildPageCss/buildRotateCss always receive safe numbers; update the code path that constructs the innerHTML (the useHead call) to pass the sanitized/clamped values instead of raw state to prevent malformed CSS injection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/lib/reports/label-generator.ts`:
- Around line 228-230: The translation used to recenter rotated labels
incorrectly reuses the 90° vector for 270°; update the transform calculation in
the function that builds the CSS string (where shift and rotation are used and
the class .maker-label is returned) to invert the shift for 270°. Compute
translateX and translateY based on rotation (e.g., for rotation===90 use
translateX = -shift, translateY = shift; for rotation===270 use translateX =
shift, translateY = -shift; for 0/180 use 0 as appropriate) and then use those
variables in the translate(...) portion of the returned transform instead of
always using (-shift, shift).
- Line 141: The vertical gap calculation for rows uses page.height but should
use the printable area height; update the gapY computation in label-generator.ts
(where gapY is defined using rows, cardHeight) to use availablePageHeight
instead of page.height so top/bottom padding is honored and spacing is computed
as (availablePageHeight - rows * cardHeight) / (rows - 1) when rows > 1 (or 0
otherwise).
In `@frontend/pages/reports/label-generator.vue`:
- Around line 318-321: The calcPages() failure path currently returns early but
leaves stale pages visible/printable; update calcPages() so when result.ok is
false it clears the pages state (e.g., set pages = [] or pages.value = []) and
also set a flag or return a falsy value indicating calculation failed; then
update printLabels() to check that pages is non-empty (or the calc success flag)
before calling window.print() and show the toast instead of printing when no
valid pages exist. Ensure you reference and modify the existing calcPages(),
printLabels(), pages variable and the result handling so a failed geometry
recalculation both clears pages and prevents window.print() from executing.
---
Nitpick comments:
In `@frontend/lib/reports/label-generator.test.ts`:
- Around line 176-181: Add a new test that asserts buildRotateCss("maker", size,
270) produces the symmetric recentering CSS (mirror of the 90° case) so the 270°
rotation branch is covered; locate the test block in label-generator.test.ts
near the existing 90° test and add a case named like "270 rotation sizes and
re-centers the label onto the swapped page" that calls buildRotateCss with
rotation 270 and expects the appropriate `@media` print CSS string for 270deg with
the corresponding translate offsets and transform-origin.
In `@frontend/pages/reports/label-generator.vue`:
- Around line 403-409: The generated CSS is injected via useHead innerHTML using
buildPageCss and buildRotateCss with values from mode.value, makerSize.value and
printRotation.value; before calling those functions, validate and clamp any
numeric inputs (e.g., labelWidth, labelHeight, labelsPerRow, labelGap, rotation)
taken from makerSize or printRotation to finite, reasonable bounds (use
Number.isFinite checks and min/max limits) and normalize types (parseFloat/Int)
so buildPageCss/buildRotateCss always receive safe numbers; update the code path
that constructs the innerHTML (the useHead call) to pass the sanitized/clamped
values instead of raw state to prevent malformed CSS injection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9ce2fed9-65fe-4de3-b09e-5f112d3cf539
📒 Files selected for processing (4)
frontend/lib/reports/label-generator.test.tsfrontend/lib/reports/label-generator.tsfrontend/locales/en.jsonfrontend/pages/reports/label-generator.vue
Comment on lines
+228
to
+230
| const shift = (size.width - size.height) / 2; | ||
| return `@media print { .maker-label { width: ${size.width}${m}; height: ${size.height}${m}; transform: translate(${-shift}${m}, ${shift}${m}) rotate(${rotation}deg); transform-origin: center center; } }`; | ||
| } |
Contributor
There was a problem hiding this comment.
270° recentering translation is mirrored incorrectly.
Line 229 applies the 90° translate vector for both 90 and 270. For 270, the shift must be inverted; otherwise labels render offset on the swapped page.
Proposed fix
const m = size.measure;
const shift = (size.width - size.height) / 2;
- return `@media print { .maker-label { width: ${size.width}${m}; height: ${size.height}${m}; transform: translate(${-shift}${m}, ${shift}${m}) rotate(${rotation}deg); transform-origin: center center; } }`;
+ const tx = rotation === 270 ? shift : -shift;
+ const ty = rotation === 270 ? -shift : shift;
+ return `@media print { .maker-label { width: ${size.width}${m}; height: ${size.height}${m}; transform: translate(${tx}${m}, ${ty}${m}) rotate(${rotation}deg); transform-origin: center center; } }`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/lib/reports/label-generator.ts` around lines 228 - 230, The
translation used to recenter rotated labels incorrectly reuses the 90° vector
for 270°; update the transform calculation in the function that builds the CSS
string (where shift and rotation are used and the class .maker-label is
returned) to invert the shift for 270°. Compute translateX and translateY based
on rotation (e.g., for rotation===90 use translateX = -shift, translateY =
shift; for rotation===270 use translateX = shift, translateY = -shift; for 0/180
use 0 as appropriate) and then use those variables in the translate(...) portion
of the returned transform instead of always using (-shift, shift).
Used the code rabbit requested changes. I feel these are mostly trivial.
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.
What type of PR is this?
What this PR does / why we need it:
Cleans up the label generator, mostly around the label maker (continuous tape) mode so it actually prints right on a real label printer. I have a Brother QL-820NWB with DK-2205 tape and was fighting the output, so a few of these are things I needed to make it usable.
Which issue(s) this PR fixes:
(none)
Special notes for your reviewer:
Worth knowing: Chrome does not reliably honor @page { size } for label printers (chromium bug 238303), so the real orientation control lives in the OS "Print using system dialog". The rotation dropdown is there for printers that rotate the page, but on my QL the actual fix was setting the printer orientation in the system dialog, which is why I added the hint in the maker instructions.
The typecheck on main is already throwing a pile of pre-existing errors (the EntitySummary/location stuff). This PR does not add to that.
Testing