feat(heroes): Add hero background images support#377
Merged
Conversation
Contributor
Reviewer's GuideAdds support for hero background images by introducing a new BackgroundImageSize enum and HeroBackground model, wiring them into the Hero schema, and extending the Blizzard hero parser to extract background images and their responsive breakpoints. Class diagram for new hero background image supportclassDiagram
class BaseModel
class Role
class BackgroundImageSize {
<<enum>>
MIN
XS
SM
MD
LG
XL_PLUS
}
class HeroBackground {
+HttpUrl url
+list~BackgroundImageSize~ sizes
}
class Hero {
+str name
+str description
+list~HeroBackground~ backgrounds
+Role role
+list~str~ images
}
Hero *-- HeroBackground : backgrounds
HeroBackground --> BackgroundImageSize : sizes
Hero --> Role : role
HeroBackground ..> BaseModel
Hero ..> BaseModel
Flow diagram for updated hero summary parsing with backgroundsflowchart TD
A[overview_section input] --> B[Extract header_section and extra_list_items]
B --> C[Extract icon_element and icon_url]
B --> D[Select blz-image elements with slot=background]
D --> E[Filter images with src attribute]
E --> F[For each image, read src as url]
F --> G[Read bp attribute, split into sizes list]
G --> H[Build list of background dicts url and sizes]
B --> I[Extract name and description]
B --> J[Extract location and birthday]
C --> K[Derive role from icon_url]
I --> L[Assemble hero summary dict]
J --> L
K --> L
H --> L
L[Return hero summary with name, description, backgrounds, role, location, birthday]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `app/heroes/models.py:10-17` </location>
<code_context>
+from .enums import BackgroundImageSize, HeroGamemode, HeroKey, MediaType
+
+
+class HeroBackground(BaseModel):
+ url: HttpUrl = Field(
+ ...,
+ description="URL of the background image",
+ examples=[
+ "https://blz-contentstack-images.akamaized.net/v3/assets/blt2477dcaf4ebd440c/blt242e79efb1e27251/631a8b791566e20e82f30288/1600_Cassidy.jpg",
+ ],
+ )
+ sizes: list[BackgroundImageSize] = Field(
+ ...,
</code_context>
<issue_to_address>
**suggestion:** Consider enforcing at least one breakpoint size per HeroBackground entry.
As defined, `sizes: list[BackgroundImageSize]` allows an empty list, so a background may not be usable at any breakpoint. Consider enforcing `min_items=1` (or equivalent) so each `HeroBackground` is guaranteed to apply to at least one size.
</issue_to_address>
### Comment 2
<location> `app/adapters/blizzard/parsers/hero.py:102-109` </location>
<code_context>
icon_element = extra_list_items[0].css_first("blz-icon")
icon_url = safe_get_attribute(icon_element, "src")
+ backgrounds = [
+ {
+ "url": img.attributes["src"],
+ "sizes": (img.attributes.get("bp") or "").split(),
+ }
+ for img in overview_section.css("blz-image[slot=background]")
+ if img.attributes.get("src")
+ ]
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Skip background images that have no breakpoint sizes instead of returning entries with an empty sizes list.
This will add backgrounds for any `blz-image[slot=background]` with a `src`, even when `bp` is missing, producing entries with `sizes: []`. If every background is expected to have at least one breakpoint, consider also requiring a non-empty `bp` in the comprehension (e.g. `if img.attributes.get("src") and img.attributes.get("bp")`) or filtering out entries whose computed `sizes` is empty so unusable background objects aren’t created.
```suggestion
backgrounds = [
{
"url": img.attributes["src"],
"sizes": (img.attributes.get("bp") or "").split(),
}
for img in overview_section.css("blz-image[slot=background]")
if img.attributes.get("src") and img.attributes.get("bp")
]
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Summary by Sourcery
Add structured support for hero background images across models and Blizzard hero parsing.
New Features: