feat: add preferences.qrCode for a header QR matrix linking to basics.url#126
feat: add preferences.qrCode for a header QR matrix linking to basics.url#126smur89 wants to merge 5 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an optional ChangesHeader QR Code Feature
Sequence Diagram(s)sequenceDiagram
participant User as CV Author
participant alta as alta()
participant qr as internal/qr.typ
participant header as _header()
participant zebra as `@preview/zebra`
User->>alta: preferences.qrCode: "auto" | string | none
alta->>qr: _check_qr_code(preferences.qrCode)
qr-->>alta: validated (or panic)
alta->>header: _header(..., qr-code: preferences.qrCode)
header->>qr: _resolve_qr_url(qr-code, basics)
qr-->>header: resolved URL or none
alt qr-url != none
header->>qr: _qr_render(url, accent, size)
qr->>zebra: qrcode(url, quiet-zone: 0, width)
zebra-->>qr: QR matrix
qr-->>header: link(url, QR matrix)
header-->>alta: header with portrait + QR positioned
else
header-->>alta: header without QR
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@internal/qr.typ`:
- Around line 79-82: The `qrcode` function call within the `_qr_render` function
is using individual named parameters (width, quiet-zone, fill) as separate
arguments, but the zebra API signature requires these options to be passed as a
dictionary in the second argument. Fix this by wrapping all the named parameters
(width: size, quiet-zone: 0, fill: fill) into a single dictionary argument,
changing the qrcode call to pass the dictionary as the second parameter instead
of spreading the options as individual named arguments.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: defc8269-7945-4199-87b7-de6060842fa9
⛔ Files ignored due to path filters (1)
examples/tests/header_qr_code.pdfis excluded by!**/*.pdf
📒 Files selected for processing (6)
README.mdinternal/header.typinternal/layout.typinternal/qr.typlib.typtests/header_qr_code.typ
| #let _qr_render(url, size, fill) = link( | ||
| url, | ||
| qrcode(url, width: size, quiet-zone: 0, fill: fill), | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify zebra package API for qrcode function.
# Expected: qrcode function signature accepts width, quiet-zone, fill parameters.
# Search for qrcode function definition in zebra package
rg -nP --type=typst -C5 'let qrcode\s*=' || echo "No qrcode definition found with rg"
# Alternatively, search for parameter usage patterns
rg -nP --type=typst 'width:|quiet-zone:|fill:' || echo "No parameter patterns found"Repository: smur89/alta-typst
Length of output: 202
🌐 Web query:
What parameters does the qrcode function accept in Typst package @preview/zebra version 0.1.0?
💡 Result:
In the Typst package @preview/zebra version 0.1.0, the qrcode function accepts the following parameters [1][2]: Positional Parameter: - data: str | bytes - The data to be encoded in the QR code [1][2]. When using a string with non-ASCII characters, an ECI UTF-8 designator is added automatically unless the data is provided as bytes [1][2]. Named Parameter: - options: dictionary - A dictionary containing barcode-specific encoding options [1][2]. The default is empty, (:). The available keys within this dictionary include: - quiet-zone: int | bool | none - Sets the quiet zone (default: 0) [1][2]. When set to true, the recommended quiet zone for QR codes (4) is added [1][2]. - width or height or module-size: auto | length - Specifies the size (default: module-size: 3pt) [1][2]. The module-size represents the size of a single black or white square, and the others are calculated automatically [1][2]. - fill: color | gradient - The fill color of the code (default: black) [1][2]. - background-fill: none | color | gradient - The background color (default: none) [1][2]. - versions: "normal" | "micro" - Specifies the QR code version (default: "normal") [1][2]. - mode: none | "numeric" | "alphanumeric" | "byte" - The character set used to encode the data; "none" automatically determines the best encoding [1][2]. - ec-level: "l" | "m" | "q" | "h" - The error correction level (default: "m") [1][2].
Citations:
🏁 Script executed:
#!/bin/bash
# Check the full qr.typ file to understand the context
cat -n internal/qr.typ | head -100
# Search for other qrcode function calls
rg -n "qrcode" internal/qr.typRepository: smur89/alta-typst
Length of output: 4217
Fix qrcode function call syntax to match zebra API.
The qrcode function from @preview/zebra:0.1.0 expects a dictionary of options as its second argument, not individual named parameters. The current call qrcode(url, width: size, quiet-zone: 0, fill: fill) is incompatible with the zebra API signature qrcode(data, options: dictionary).
Change line 81 to:
qrcode(url, (width: size, quiet-zone: 0, fill: fill))🤖 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 `@internal/qr.typ` around lines 79 - 82, The `qrcode` function call within the
`_qr_render` function is using individual named parameters (width, quiet-zone,
fill) as separate arguments, but the zebra API signature requires these options
to be passed as a dictionary in the second argument. Fix this by wrapping all
the named parameters (width: size, quiet-zone: 0, fill: fill) into a single
dictionary argument, changing the qrcode call to pass the dictionary as the
second parameter instead of spreading the options as individual named arguments.
….url Adds `preferences.qrCode` — opt-in, default `none` — which renders a small accent-coloured QR matrix in the header on the side opposite the portrait. `qrCode: "url"` encodes `basics.url`; any other string is encoded verbatim, useful when the printed CV should point at a tracked landing page distinct from the canonical homepage. Closes #59. QR generation is delegated to `@preview/zebra:0.1.0` (MIT, ~170 LOC + one WASM blob, no transitive Typst deps, emits native vector paths rather than rasterised images). This is the first third-party Typst package altacv depends on, so callers will see a one-time download on first compile after upgrading. Layout: with `imagePosition: "right"` the QR lands on the left, photo on the right, header text in the middle. With no portrait, the QR still lands on the side opposite `imagePosition`. With `imagePosition: "center"`, the photo stays stacked above/below the text block and the QR sits beside the text row. The matrix is wrapped in `link()` so a digital PDF reader can click through to the same destination the QR encodes — the QR is for print, the click is for screen. Implementation lives in a new `internal/qr.typ` module so the zebra import stays a one-file change if a different backend becomes preferable.
…omments - Rename the `preferences.qrCode: "url"` sentinel to `preferences.qrCode: auto`. `auto` is Typst's idiomatic 'derive from context' value and cleanly splits the API into `none` / `auto` / `"<verbatim URL>"`, freeing the literal three-letter string `"url"` for the verbatim path. - Centred-photo + QR: switch the text-row grid from `(1fr, auto)` to `(1fr, auto, 1fr)` with an empty spacer mirroring the QR's column so the header text stays page-centred under `headerTextAlign: "center"` instead of drifting left of the page midline. - Add a 7th `tests/header_qr_code.typ` fixture exercising the no-portrait + `imagePosition: "center"` path through the renderer (centred-photo slot empty, QR rides the text row alone). - Regenerate `examples/tests/header_qr_code.pdf` against the renamed sentinel + the new fixture so the CI sync guard stays green. - Trim the `qrCode` default-preference comment in `internal/layout.typ` and the `_qr_render` docstring in `internal/qr.typ` to match the brevity of peer prefs / helpers.
smur89
force-pushed
the
feat/issue-59-header-qr-code-v2
branch
from
7c2654f to
b8ed69d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@internal/qr.typ`:
- Around line 30-33: The qrCode function currently treats the string "url" as a
literal payload instead of resolving it to basics.url. At line 30-33, add a
special case check for when value equals the string "url" to resolve it via
basics.url, similar to how auto is handled. Apply the same fix at line 47-48
where similar validation logic exists. When the sentinel value "url" is
detected, the code should retrieve and use the actual URL from basics.url rather
than passing the literal string "url" as the QR code payload.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1f991094-0ead-40e4-af4d-69e9b0e523f8
⛔ Files ignored due to path filters (1)
examples/tests/header_qr_code.pdfis excluded by!**/*.pdf
📒 Files selected for processing (6)
README.mdinternal/header.typinternal/layout.typinternal/qr.typlib.typtests/header_qr_code.typ
| if value == none or value == auto { return } | ||
| if type(value) != str { | ||
| panic( | ||
| "qrCode must be `none`, `auto`, or a URL string, got: " + repr(value), |
There was a problem hiding this comment.
"url" sentinel is currently treated as a literal payload, not as basics.url.
On Line 30 and Line 47, the code only special-cases auto. Per the stated PR objective, preferences.qrCode: "url" should resolve via basics.url; today it falls through as a verbatim string and generates the wrong QR/link target.
Suggested minimal fix
`#let` _check_qr_code(value) = {
- if value == none or value == auto { return }
+ if value == none or value == "url" or value == auto { return }
if type(value) != str {
panic(
"qrCode must be `none`, `auto`, or a URL string, got: " + repr(value),
)
}
if value == "" {
panic("qrCode must be a non-empty string when not `none` / `auto`.")
}
}
`#let` _resolve_qr_url(qr-code, basics) = {
if qr-code == none { return none }
- if qr-code != auto { return qr-code }
+ if qr-code != auto and qr-code != "url" { return qr-code }
let url = basics.at("url", default: none)Also applies to: 47-48
🤖 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 `@internal/qr.typ` around lines 30 - 33, The qrCode function currently treats
the string "url" as a literal payload instead of resolving it to basics.url. At
line 30-33, add a special case check for when value equals the string "url" to
resolve it via basics.url, similar to how auto is handled. Apply the same fix at
line 47-48 where similar validation logic exists. When the sentinel value "url"
is detected, the code should retrieve and use the actual URL from basics.url
rather than passing the literal string "url" as the QR code payload.
- Page 8: centred portrait + QR + `imageStackOrder: "below"`. Verifies the photo trails the text-row instead of preceding it, and the QR still rides the text-row beside the contact info. - Page 9: centred portrait + QR + default `headerTextAlign: "left"`. The centred-photo axis stays page-centred while the header text + QR compose left-aligned in their row — exercises the orthogonality of `imagePosition` and `headerTextAlign` documented in the README.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/header_qr_code.typ`:
- Around line 151-154: Scenario 9 currently relies on the default
headerTextAlign value, which makes the test fragile if defaults change. In the
preferences object (which contains qrCode and imagePosition properties),
explicitly add the headerTextAlign property set to "left" to make the test
contract stable and ensure it continues testing the intended orthogonal behavior
regardless of future default changes.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 021345a7-c287-4e2c-8165-025685389787
⛔ Files ignored due to path filters (1)
examples/tests/header_qr_code.pdfis excluded by!**/*.pdf
📒 Files selected for processing (1)
tests/header_qr_code.typ
| preferences: ( | ||
| qrCode: auto, | ||
| imagePosition: "center", | ||
| ), |
There was a problem hiding this comment.
Make scenario 9 explicitly set left text alignment.
This case currently relies on the default headerTextAlign value, so it may stop testing the intended orthogonal behaviour if defaults change. Set it explicitly to keep the test contract stable.
Proposed patch
preferences: (
qrCode: auto,
imagePosition: "center",
+ headerTextAlign: "left",
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| preferences: ( | |
| qrCode: auto, | |
| imagePosition: "center", | |
| ), | |
| preferences: ( | |
| qrCode: auto, | |
| imagePosition: "center", | |
| headerTextAlign: "left", | |
| ), |
🤖 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 `@tests/header_qr_code.typ` around lines 151 - 154, Scenario 9 currently relies
on the default headerTextAlign value, which makes the test fragile if defaults
change. In the preferences object (which contains qrCode and imagePosition
properties), explicitly add the headerTextAlign property set to "left" to make
the test contract stable and ensure it continues testing the intended orthogonal
behavior regardless of future default changes.
- Frame 6 (plum + portrait right) gains `qrCode: auto`, producing the canonical [QR | header text | portrait] arrangement so the new print-affordance feature is visible in the animated README hero alongside accent / layout / date-format variations. - Update the README gif alt text to mention "header QR code" so screen readers and search indexes pick up the addition. - Regenerate `examples/preview.gif` against the new frame definition.
- Frame 4 (crimson + portrait left + Irish labels) gains `qrCode: auto`, producing the mirror of frame 6's layout: portrait on the left, QR on the right. Two QR-bearing frames in the gif — one under each `imagePosition` — give the print-affordance feature a visible cycle alongside the accent palette rotation. - Regenerate `examples/preview.gif` against the updated frame.
1 participant
Summary
Adds
preferences.qrCode— opt-in, defaultnone— which renders a small accent-coloured QR matrix in the header on the side opposite the portrait.qrCode: autoencodesbasics.url; any non-empty string is encoded verbatim, useful when the printed CV should point at a tracked landing page distinct from the canonical homepage.Closes #59. Supersedes #88 (PR predates the
lib.typ→internal//sections/split + theinternal/labels-en.tomlextraction).Notes for the reviewer
New third-party dependency. QR generation is delegated to
@preview/zebra:0.1.0— MIT, ~170 LOC + one WASM blob, no transitive Typst deps, emits native vector paths rather than rasterised images. This is the first third-party Typst package altacv depends on, so callers will see a one-time download on first compile after upgrading. Picked zebra over@preview/cades(pulls in@preview/jogs, a JS interpreter) and@preview/tiaoma(Zint-via-WASM, larger). Worth a vet on your side.Implementation lives in
internal/qr.typso the zebra import stays a one-file change if a different backend becomes preferable.internal/header.typconsumes the resolved data via a small module API.The QR matrix is wrapped in
link()so a digital PDF reader can click through to the same destination the QR encodes — print users scan, screen users click.autosentinel, not"url". The "encodebasics.url" path uses Typst'sautokeyword — the language's idiomatic "derive from context" value — so the API splits cleanly intonone/auto/"<verbatim URL>"and the literal three-letter string"url"stays available as a valid verbatim payload.Centred-photo + QR keeps text page-centred. Under
imagePosition: "center"+headerTextAlign: "center"+ a QR, the text-row grid uses(1fr, auto, 1fr)with an empty spacer mirroring the QR column, so the header text sits on the page midline instead of drifting left of it.Test plan
make test—tests/header_qr_code.typplus all pre-existing fixtures green.autoform, no portraitimagePosition: "left"— flipped)accentheaderTextAlign: "center"imagePosition: "center"imageStackOrder: "below"headerTextAlign: "left"(orthogonal axes)examples/tests/header_qr_code.pdfregenerated to match.basics.url.Summary by CodeRabbit
Release Notes
New Features
preferences.qrCode(none,auto, or any non-empty string).auto, the QR encodesbasics.url; otherwise the provided value is used verbatim.imagePosition, including centred header layouts and accent styling.Documentation
Tests