feat: add preferences.density preset to scale spacing tokens uniformly#87
feat: add preferences.density preset to scale spacing tokens uniformly#87smur89 wants to merge 3 commits into
Conversation
smur89
force-pushed
the
feat/issue-50-density-preset
branch
5 times, most recently
from
9fb0935 to
2b0b958
Compare
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughIntroduces a ChangesDensity Preference Feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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 `@lib.typ`:
- Around line 327-334: The _labelled_divider() function at lines 346 and 359
uses unscaled vertical spacing v(0.3 * body-size), while the divider() function
shown in the diff now uses scaled spacing v(0.3 * scale * body-size). To
maintain consistency within the same section, update _labelled_divider() to
retrieve the scale factor using _spacing_scale_state.get() and apply it to both
vertical spacing calls, matching the pattern used in divider() where the
vertical spacing is multiplied by both the scale factor and body-size.
🪄 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: b54662c8-d827-4400-92dc-17a864e7b82e
⛔ Files ignored due to path filters (1)
examples/tests/density.pdfis excluded by!**/*.pdf
📒 Files selected for processing (3)
README.mdlib.typtests/density.typ
| let scale = _spacing_scale_state.get() | ||
| v(0.3 * scale * body-size) | ||
| line( | ||
| length: 100%, | ||
| stroke: (paint: _divider_colour, thickness: 0.6pt, dash: "dashed"), | ||
| ) | ||
| v(0.3 * body-size) | ||
| v(0.3 * scale * body-size) | ||
| } |
There was a problem hiding this comment.
Scale consistency gap: _labelled_divider() vertical spacing is still unscaled.
You scaled divider() here, but grouped-certificate separators still keep fixed density because _labelled_divider() uses v(0.3 * body-size) at Line 346 and Line 359. This makes density behavior inconsistent within the same section.
Suggested patch
`#let` _labelled_divider(label) = context {
let body-size = _body_size_state.get()
+ let scale = _spacing_scale_state.get()
let stroke = (paint: _divider_colour, thickness: 0.6pt, dash: "dashed")
- v(0.3 * body-size)
+ v(0.3 * scale * body-size)
pad(left: 0.6 * body-size, grid(
columns: (1.3em, auto, 1fr),
column-gutter: 0.5 * body-size,
align: horizon,
@@
line(length: 100%, stroke: stroke),
))
- v(0.3 * body-size)
+ v(0.3 * scale * body-size)
}🤖 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 `@lib.typ` around lines 327 - 334, The _labelled_divider() function at lines
346 and 359 uses unscaled vertical spacing v(0.3 * body-size), while the
divider() function shown in the diff now uses scaled spacing v(0.3 * scale *
body-size). To maintain consistency within the same section, update
_labelled_divider() to retrieve the scale factor using
_spacing_scale_state.get() and apply it to both vertical spacing calls, matching
the pattern used in divider() where the vertical spacing is multiplied by both
the scale factor and body-size.
2b0b958 to
1108dcb
Compare
smur89
force-pushed
the
feat/issue-50-density-preset
branch
from
b1e3979 to
2d7a59b
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 `@lib.typ`:
- Around line 1153-1158: The block element wrapping the project description uses
a hardcoded `0.6em` value for the below spacing, which ignores the density-aware
spacing model used in other parts of the code like the `_experience` and
`_education` functions. Replace the fixed `0.6em` value in the `block(below:
0.6em, emph(description))` call with a variable or expression that respects the
compact/spacious density settings, matching the spacing approach used
consistently throughout the document for similar layout elements.
🪄 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: f978eec6-8b71-46e5-a3a6-8fdc5431a8cb
⛔ Files ignored due to path filters (2)
examples/preview.pngis excluded by!**/*.pngexamples/tests/density.pdfis excluded by!**/*.pdf
📒 Files selected for processing (3)
README.mdlib.typtests/density.typ
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 `@lib.typ`:
- Around line 1153-1158: The block element wrapping the project description uses
a hardcoded `0.6em` value for the below spacing, which ignores the density-aware
spacing model used in other parts of the code like the `_experience` and
`_education` functions. Replace the fixed `0.6em` value in the `block(below:
0.6em, emph(description))` call with a variable or expression that respects the
compact/spacious density settings, matching the spacing approach used
consistently throughout the document for similar layout elements.
🪄 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: f978eec6-8b71-46e5-a3a6-8fdc5431a8cb
⛔ Files ignored due to path filters (2)
examples/preview.pngis excluded by!**/*.pngexamples/tests/density.pdfis excluded by!**/*.pdf
📒 Files selected for processing (3)
README.mdlib.typtests/density.typ
🛑 Comments failed to post (1)
lib.typ (1)
1153-1158:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winScale the project description gap.
Line 1157 still uses a fixed
0.6em, so compact/spacious density won't affect project descriptions. This is a small but visible gap in the new spacing model.♻️ Proposed fix
- block(below: 0.6em, emph(description)) + context { + let scale = _spacing_scale_state.get() + block(below: 0.6 * scale * 1em, emph(description)) + }📝 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.// Wrapped in a block so the gap to the term row below matches // the institution-line → term spacing in `_experience` / // `_education`; using a bare `linebreak()` here leaves no // paragraph spacing. context { let scale = _spacing_scale_state.get() block(below: 0.6 * scale * 1em, emph(description)) }🤖 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 `@lib.typ` around lines 1153 - 1158, The block element wrapping the project description uses a hardcoded `0.6em` value for the below spacing, which ignores the density-aware spacing model used in other parts of the code like the `_experience` and `_education` functions. Replace the fixed `0.6em` value in the `block(below: 0.6em, emph(description))` call with a variable or expression that respects the compact/spacious density settings, matching the spacing approach used consistently throughout the document for similar layout elements.
|
Superseded by #127 — rebuilt against the post-refactor module layout, plus a latent fix in |
1 participant
Summary
Adds
preferences.density— a single coherent knob that scales every spacing em-token uniformly, replacing the need for users to hand-tunebodySize,margin, and individual em-multipliers when they want a tighter (one-page fit) or roomier (print presentation) layout. Three presets:"compact"(× 0.85),"comfortable"(× 1.0, default),"spacious"(× 1.15). Closes #50.The multiplier is threaded through a new
_spacing_scale_stateand applied at every blockabove/below, everyv(),par.spacing/leading, andlist.spacing. Text sizes, icon dimensions, and rating-dot geometry are intentionally left alone — density is purely about vertical whitespace, so font-size scaling stays underbodySize.Rendered output
Default (
"comfortable") is byte-identical to main — verified viatypst compile --format pngonexamples/example.typbefore/after. The newtests/density.typfixture renders all three presets side-by-side over three pages; CI will upload the artifact.Test plan
typst compile --root . examples/example.typ examples/example.pdfsucceedstests/*.typfixture compiles (including the newtests/density.typ)tests/density.typexercising compact / comfortable / spaciousPreferencestable with the new keycmponexamples/example.typ)Notes for the reviewer
_strict_mergestyle.v(-0.7 * scale * body-size)inside the L2 heading show-rule is scaled along with everything else; visual check confirms the accent-rule anchoring stays clean at both 0.85 and 1.15 (no overlap, no excessive gap).set par(leading: 0.55 * scale * 1em, ...)form was chosen over0.55em * scalepurely to mirror theK * scale * body-sizepattern used elsewhere; numerically identical.Summary by CodeRabbit
New Features
preferences.densitysetting with presets:compact,comfortable, andspacious.Documentation
density, its default (comfortable), and supported values.Tests