chore: shift document cover by offset #211
Open
Conversation
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
Reviewer's GuideAdds support for a normalized vertical offset on document cover images and propagates this offset through view metadata, database row metadata, and rendering components so covers are clamped and displayed with the correct object position across web, mobile, database, and publish views. Sequence diagram for updating and rendering a document cover with offsetsequenceDiagram
actor User
participant ViewCoverActions
participant ViewMetaPreview
participant CoverUtils
participant Backend
participant ViewCoverComponent
participant ImageRender
User->>ViewCoverActions: select new cover (type, value)
ViewCoverActions->>ViewMetaPreview: onUpdateCover( newCover )
ViewMetaPreview->>CoverUtils: clampCoverOffset( newCover.offset )
CoverUtils-->>ViewMetaPreview: clampedOffset
ViewMetaPreview->>ViewMetaPreview: normalizeCover( newCover )
ViewMetaPreview->>Backend: updatePage( viewId, extra.cover{ type, value, offset } )
Backend-->>ViewMetaPreview: success
User->>ViewMetaPreview: view updated document
ViewMetaPreview->>ViewCoverComponent: render cover with coverOffset
ViewCoverComponent->>CoverUtils: coverOffsetToObjectPosition( offset )
CoverUtils-->>ViewCoverComponent: objectPosition
ViewCoverComponent->>ImageRender: render image with style.objectPosition
ImageRender-->>User: cover shown with correct vertical offset
Class diagram for updated cover-related types and componentsclassDiagram
class ViewCover {
+CoverType type
+string value
+number offset
}
class ViewMetaCover {
}
class RowMetaCover {
+string data
+RowCoverType cover_type
+number offset
}
class RowMeta {
+RowMetaCover cover
+string icon
+boolean isEmptyDocument
}
class ViewExtra {
+ViewCover cover
}
class UpdatePagePayload {
+string name
+ViewExtra extra
}
class ViewMetaPreview {
-ViewMetaCover cover
-ViewMetaCover normalizeCover( ViewMetaCover cover )
-void handleUpdateCover( ViewMetaCover newCover )
}
class ViewCoverComponent {
+string coverValue
+string coverType
+number coverOffset
}
class MobileRecentViewCover {
+ViewCover cover
}
class DatabaseRowHeader {
-void setRowCover( ViewCover cover )
-RowMeta rowMeta
}
class PublishViewMetaHook {
+ViewMetaCover cover
}
class CoverUtils {
<<utility>>
+number clampCoverOffset( number offset )
+string coverOffsetToObjectPosition( number offset )
}
class ImageRender {
+string src
+object style
}
ViewMetaCover --|> ViewCover
ViewExtra --> ViewCover
UpdatePagePayload --> ViewExtra
ViewMetaPreview --> ViewMetaCover
ViewMetaPreview ..> CoverUtils
ViewMetaPreview ..> ViewCoverComponent
ViewCoverComponent --> ViewCover
ViewCoverComponent ..> ImageRender
ViewCoverComponent ..> CoverUtils
MobileRecentViewCover --> ViewCover
MobileRecentViewCover ..> ImageRender
MobileRecentViewCover ..> CoverUtils
DatabaseRowHeader --> RowMeta
DatabaseRowHeader ..> CoverUtils
RowMeta --> RowMetaCover
PublishViewMetaHook --> ViewMetaCover
PublishViewMetaHook ..> CoverUtils
ImageRender <.. DatabaseRowHeader
File-Level Changes
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}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
row_meta.tsyou’re now callingclampCoverOffsetinsidegetMetaJSON, but there’s no corresponding import added in that file, which will cause a compile error; add the missing import from@/utils/cover. - You now normalize cover offsets both in
ViewMetaPreview(vianormalizeCover) and inuseViewMeta; consider centralizing this normalization (e.g., a shared helper that returns a normalizedViewCover) to avoid duplicated logic and ensure consistent behavior across entry points.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `row_meta.ts` you’re now calling `clampCoverOffset` inside `getMetaJSON`, but there’s no corresponding import added in that file, which will cause a compile error; add the missing import from `@/utils/cover`.
- You now normalize cover offsets both in `ViewMetaPreview` (via `normalizeCover`) and in `useViewMeta`; consider centralizing this normalization (e.g., a shared helper that returns a normalized `ViewCover`) to avoid duplicated logic and ensure consistent behavior across entry points.
## Individual Comments
### Comment 1
<location> `src/components/_shared/image-render/ImageRender.tsx:23-26` </location>
<code_context>
+ height: loading ? 0 : '100%',
+ width: loading ? 1 : '100%',
+ };
+ const mergedStyle = {
+ ...baseStyle,
+ ...style,
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Allowing caller `style` to override internal error/loading styles could break expected behavior.
Because `mergedStyle` spreads `baseStyle` first and then `style`, callers can override `display`, `height`, and `width`. For instance, `style={{ display: 'block' }}` would cause the image to render even when `hasError` is true, bypassing error handling; similar overrides could break the loading skeleton. Consider reversing the spread order (`{ ...style, ...baseStyle }`) or explicitly locking down key properties like `display`, `height`, and `width` so external styles can’t violate the component’s core behavior.
```suggestion
const mergedStyle = {
...style,
...baseStyle,
};
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Checklist
General
Testing
Feature-Specific
Summary by Sourcery
Support normalized cover offset for document and database covers to control image positioning across views.
New Features:
Enhancements: