feat: add color customization support for templates with validation#35
feat: add color customization support for templates with validation#35
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughTemplates now include per-template color definitions. Request parameters can supply validated color overrides; generate_svg requires an explicit template name and applies overrides to template content before parsing to produce the final SVG. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Route as routes/index.rs
participant Params as params.rs
participant Templates as templates.rs
participant Generator as generator/svg.rs
Client->>Route: Request with OgParams (+extra color params)
Route->>Params: extract_colors(template_name, state)
Params->>Templates: lookup template color definitions
Templates-->>Params: allowed color keys & defaults
alt Colors valid
Params-->>Route: color_overrides (normalized, `#`-prefixed)
Route->>Generator: generate_svg(..., template_name, color_overrides)
Generator->>Templates: fetch template content by name
Generator->>Generator: override_colors(content, template_name, templates, color_overrides)
Generator->>Generator: parse SVG from overridden content
Generator-->>Route: SVG string
Route-->>Client: 200 OK with SVG
else Colors invalid
Params-->>Route: Err (invalid hex / unknown key)
Route-->>Client: 400 Bad Request with error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for dynamic color customization in SVG templates. Users can now override template colors via query parameters, enabling personalized branding without creating new templates.
- Added color definitions to all templates in
templates.yaml - Implemented color extraction, validation, and override functionality
- Modified SVG generation to apply color replacements
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| templates.yaml | Added color definitions for each template (background, text colors, accents, etc.) |
| src/templates.rs | Added color parsing and storage logic to load color definitions from template YAML |
| src/params.rs | Added extra field for color parameters with validation logic using regex |
| src/routes/index.rs | Integrated color extraction and validation into the request pipeline |
| src/generator/svg.rs | Implemented color override logic using string replacement |
| Cargo.toml | Added regex dependency (version 1.11) |
| Cargo.lock | Updated lock file with regex dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/params.rs (1)
3-101: Avoid recompiling the hex regex on every request.
Regex::newruns for each call toextract_colors, which adds measurable overhead on hot paths. AOnceLockkeeps the pattern compiled once and reuses it safely across requests.Apply this diff to cache the regex:
use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; use regex::Regex; use serde::Deserialize; use std::collections::HashMap; +use std::sync::OnceLock; use utoipa::{IntoParams, ToSchema}; use crate::AppState; use crate::config::ImageFallbackBehavior; use crate::image::ValidatedImage; +static HEX_COLOR_REGEX: OnceLock<Regex> = + OnceLock::new(|| Regex::new(r"^[0-9A-Fa-f]{6}$").unwrap()); + impl OgParams { @@ - let template_colors = state.templates.colors.get(template_name); - let hex_regex = Regex::new(r"^[0-9A-Fa-f]{6}$").unwrap(); + let template_colors = state.templates.colors.get(template_name); + let hex_regex = HEX_COLOR_REGEX.get_or_init(|| { + Regex::new(r"^[0-9A-Fa-f]{6}$").expect("hex color regex should compile") + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Cargo.toml(1 hunks)src/generator/svg.rs(1 hunks)src/params.rs(3 hunks)src/routes/index.rs(1 hunks)src/templates.rs(3 hunks)templates.yaml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/generator/svg.rs (1)
src/templates.rs (2)
available_templates(13-19)templates(14-17)
src/routes/index.rs (1)
src/generator/svg.rs (1)
generate_svg(46-104)
src/templates.rs (1)
src/yaml_loader.rs (2)
load_text(34-40)load_yaml(7-16)
🔇 Additional comments (6)
Cargo.toml (1)
20-20: Dependency addition looks good.Adding
regex = "1.11"cleanly supports the new validation path in params without affecting existing deps.templates.yaml (1)
6-70: Palette definitions align with the override workflow.The quoted hex literals ensure YAML keeps the
#, and the keys match the validator expectations—nicely coordinated.src/routes/index.rs (1)
145-175: Color override integration looks solid.Nice job threading
template_nameand the validatedcolor_overridesthrough to the generator while preserving existing error handling and logging.src/templates.rs (1)
22-115: Template color parsing is well integrated.The helper cleanly filters non-string entries and only stores non-empty palettes, keeping TemplateMap consistent with the new override flow.
src/generator/svg.rs (2)
12-24: LGTM: Clean template lookup with helpful error messages.The refactoring to require an explicit
template_nameparameter removes ambiguity and provides clear error messages when templates are not found.
46-57: All call sites updated correctly.The call at
src/routes/index.rs:166-175matches the new signature exactly, passing all 8 arguments in the correct order and types. No other files callgenerate_svg. Signature change is fully integrated.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/params.rs:55
- The
is_empty()method should also checkself.templateandself.extrafields. Currently, a request with only color parameters or only a template parameter would be considered 'empty', which could lead to incorrect default value application.
fn is_empty(&self) -> bool {
self.title.is_none()
&& self.description.is_none()
&& self.subtitle.is_none()
&& self.logo.is_none()
&& self.image.is_none()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary by CodeRabbit
New Features
Bug Fixes