Skip to content

Layouts: a publication sheet's accent must be opaque; a column always holds the legend (#682) - #750

Merged
giswqs merged 1 commit into
opengeos:mainfrom
elpidioassinaturas:fix/682-accent-opaque
Oct 23, 2026
Merged

giswqs merged 1 commit into
opengeos:mainfrom
elpidioassinaturas:fix/682-accent-opaque

Conversation

@elpidioassinaturas

@elpidioassinaturas elpidioassinaturas commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A follow-up to #742, which was merged at cb5c161 a few minutes before this last commit reached the branch. It answers two points from #742's review.

  • A translucent accent is refused. The user guide documents "accent" as #RRGGBB, but #RRGGBBAA was accepted. A translucent accent left a banner's band, a poster's page or the headings faint or unseen. layout.publication now returns an error that names the opaque form.
  • Column::legend is gone. It was true wherever a column is made, so the field and the branches for false were dead code. The locator in a column is still capped at 45% of the column's height, as before.

Testing

  • publication_layouts_take_a_style gains a case: a poster with "accent": "#1F2D4D80" is refused.
  • cargo fmt --all --check and cargo clippy -p rustgis-engine --all-targets -- -D warnings are clean.
  • rustgis-engine tests pass on current main.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Publication-layout legends are now placed through the column layout, and columns constrain locator sizing based on their height.
    • Publication accent colors with transparency are rejected with an error; fully opaque accent colors remain supported.

A translucent accent (#RRGGBBAA) would leave a banner, a poster's page or
the headings faint or unseen, so it is refused, as documented (#RRGGBB).
Column::legend was true wherever a column is made, so the field and the
branches for false are gone (opengeos#742 review).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 6d64272c-ecff-4d1b-b789-da88b075c312
📥 Commits

Reviewing files that changed from the base of the PR and between e050924 and f1547c3.

📒 Files selected for processing: 2
  • crates/engine/src/cmd/layout.rs
  • crates/engine/src/tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

📝 Walkthrough

Publication layouts now place legends in columns and limit column locator sizing. They also reject accent colors with alpha values other than 255.

Changes: Publication layout behavior

Layer / File(s) Summary
Column legend placement
crates/engine/src/cmd/layout.rs
Columns no longer carry a legend flag. Column locators are capped at 45% of column height, and legend placement uses the presence of a column.
Opaque accent validation
crates/engine/src/cmd/layout.rs, crates/engine/src/tests.rs
Publication layouts reject accents with alpha values other than 255. The test covers rejection of an eight-digit accent.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

Possibly related PRs

  • opengeos/RustGIS#742: This PR follows up on the same accent option, publication-layout test, and column legend behavior.

Suggested reviewers: giswqs

Merge Risk: ⚪ Minimal · up to f1547

No concrete current-head regression or unresolved merge-blocking risk was established.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title names two changes: a publication sheet's accent must be opaque, and a column always holds the legend. The changeset matches both. It rejects translucent accents in layout.publication and rem…
Description check Passed The description covers the Summary section with a clear account of both changes and their reasons. It explains the translucent accent rejection and the removal of Column::legend, and it notes the 45% …
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names two changes: a publication sheet's accent must be opaque, and a column always holds the legend. The changeset matches both. It rejects translucent accents in layout.publication and removes the Column::legend flag. The title's reference to #682 does not match the PR objectives, which cite #742 and #750, so the issue number may be wrong. The title is specific and reads clearly, though it covers two changes.

Full details: Description check

Explanation

The description covers the Summary section with a clear account of both changes and their reasons. It explains the translucent accent rejection and the removal of Column::legend, and it notes the 45% locator cap is unchanged. The Testing section lists the new test case, the fmt and clippy checks, and the test run. The Related issue section is missing. The description does not link an issue, and it refers to #742 only in prose.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 too large.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

A rabbit checks the map at dawn,
The column holds the legend on.
Opaque accents pass the gate,
Faded shades must change their state.
Soft paws mark the layout clean,
A tidy page shines bright and green.

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Code review

I found no bugs, security issues or guideline violations in this change. I read the diff and the surrounding code. I did not build the crate or run the tests.

What the PR does

  • layout.publication now rejects an accent that isn't fully opaque, with the error accent is an opaque colour as #RRGGBB.
  • It removes the Column::legend field. Every Column constructor set it to true, so the legend always goes in the column. side, legend_in_column and legend_rect are simplified to match.
  • It adds a regression case to publication_layouts_take_a_style: "accent": "#1F2D4D80" on a poster must fail.

Checked

  • Dead field: all three Column { … legend: true } sites are updated. c.legend is gone from side, legend_in_column and legend_rect, and the new expressions behave the same as before.
  • Accent check: it sits right after color(p, "accent", style.accent())?. Rgba::a() exists in crates/map/src/color.rs. The check applies to every style, including the default accent, which I assumed is opaque but didn't verify.
  • Never-crash rules: no unwrap, panic or indexing was added, and the error path returns Err.

Minor (low confidence)

  • Quality: the validation covers only accent. If other colour parameters of layout.publication have the same translucency problem, the check could move into a shared helper. That is outside this PR.
  • Quality: the error text says "as #RRGGBB". A named colour such as "blue" is presumably opaque and still accepted, so the wording is slightly narrower than the real rule. I didn't confirm which names color() accepts.

Change size

Compared: the whole PR against main (merge base 694e6f8 to PR head f1547c3)

Metric Value
Files changed 2 (0 added, 2 modified, 0 removed, 0 renamed)
Size before 1003.1 KB
Size after 1003.0 KB
Total size change -76 B
File Before After Change
crates/engine/src/cmd/layout.rs 155.0 KB 154.8 KB -152 B
crates/engine/src/tests.rs 848.1 KB 848.2 KB +76 B

@github-actions

Copy link
Copy Markdown

📖 Docs preview

Item Value
Preview https://preview.rustgis.app/pr-750/
Web app https://preview.rustgis.app/pr-750/app/
Commit f1547c3

GitHub Pages takes a minute or two to publish a new commit.

@giswqs
giswqs merged commit d9b2de0 into opengeos:main Oct 23, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants