vaev-style: Add running positions, content element() and counter page #79
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
Louciole
commented
Jul 16, 2025
There was a problem hiding this comment.
- remove reference of currentPage
- reorder pageLayoutInfo,
- del constructor of pageLayout info
- create Pagination context to replace args, in collection
- shortcut for pageInfos[pageContent]
- remove utility import in impl
- investigate page usage in layout
- replace Page in layout with just pageNumber
- in binary search use (search) then linear
- put RunningPos in its own object (from builder methods and layout impl)
- del constructor of RunningPositonInfo
- is lookforLayoutPosition useful cant it just be layout() ??
- customIdent should be a Symbol instead of a String
- better parsing of customIdent
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…ge to allow beautiful margins.
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for CSS running positions, content element() function, and counter page functionality to the CSS engine. The implementation includes new value types for positioning elements using running() positioning scheme and content generation through element() and counter() functions.
Key Changes
- Implements CSS running positions with
running()function support forpositionproperty - Adds
element()andcounter()functions for CSS content property - Introduces page counter support for print layout
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vaev-engine/values/primitives.cpp | Adds comparison operators and repr method to CustomIdent |
| src/vaev-engine/values/content.cpp | New file implementing ElementContent and Counter value types with parsers |
| src/vaev-engine/values/insets.cpp | Refactors Position enum to Union type and adds RunningPosition support |
| src/vaev-engine/values/defs/keywords.inc | Adds position-related keywords (Fixed, Sticky, Absolute, Relative, Static) |
| src/vaev-engine/style/specified.cpp | Updates content type from String to Content and position default value |
| src/vaev-engine/style/props.cpp | Updates ContentProp to use Content type instead of String |
| src/vaev-engine/layout/positioned.cpp | Updates position comparisons to use keyword constants |
| src/vaev-engine/layout/paint.cpp | Adds RunningPosition filtering in paint logic |
| src/vaev-engine/layout/layout-impl.cpp | Adds RunningPositionMap support and lookForRunningPosition function |
| src/vaev-engine/layout/builder.cpp | Fixes function name typo and adds support for element() and counter() content |
| src/vaev-engine/layout/base.cpp | Adds RunningPositionMap implementation and Input struct updates |
| src/vaev-engine/driver/print.cpp | Implements two-pass pagination with running position collection |
| // binary search of all running positions that match the page | ||
|
|
||
| auto res = search(list, [&](RunningPositionInfo const& info) { | ||
| if (info.page <= page and page < info.page) { |
There was a problem hiding this comment.
This condition 'info.page <= page and page < info.page' is logically impossible - no value can be both less than or equal to page AND greater than page. This will never evaluate to true.
Suggested change
| if (info.page <= page and page < info.page) { | |
| if (info.page == page) { |
Copilot uses AI. Check for mistakes.
sleepy-monax
requested changes
Jul 17, 2025
Comment on lines
+300
to
+302
| if (box.style->position.is<RunningPosition>()) { | ||
| input.fragment = nullptr; | ||
| } |
There was a problem hiding this comment.
Can't we return early instead?
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.
No description provided.