fix(app): paginate release notes by version instead of bullet - #400
Merged
Conversation
- Change MAX_RELEASE_HIGHLIGHTS from 15 to 5 (version pages, not bullets) - Introduce ParsedNotice type to distinguish bullets from summary paragraphs - Refactor parseNoticeDescriptions to return ParsedNotice | undefined - Add formatReleaseNoticeDescription to merge bullets with • prefix - Update parseRelease body path: one version = one Highlight - Keep structured highlights schema behavior unchanged - Make DialogReleaseNotes description scrollable with whitespace-pre-line - Fix title and buttons to stay fixed while description scrolls - Update tests to expect merged bullets per version page - Add regression tests for version granularity and structured schema Closes #398
Contributor
📝 WalkthroughWalkthroughRelease-note parsing now aggregates bullets per release into a single bullet-prefixed, newline-joined description and enforces separate caps: 5 release-version pages and 15 structured highlights. The release-notes dialog was refactored so the description preserves newlines, scrolls internally, and resets scroll on page changes. Tests updated accordingly. ChangesRelease Highlights Aggregation & Dialog Scroll
Sequence DiagramsequenceDiagram
participant User
participant Dialog as DialogReleaseNotes
participant Context as highlights.tsx
participant Data as ReleaseSource
User->>Dialog: open release notes
Dialog->>Context: request sliced highlights
Context->>Data: fetch/parse releases
Data-->>Context: parsed releases (structured / release-body)
Context->>Context: aggregate bullets per release, apply caps, dedupe
Context-->>Dialog: highlights (per release-body page or structured items)
Dialog->>Dialog: render title + scrollable description
User->>Dialog: navigate pages
Dialog->>Dialog: reset description scrollTop to 0 (via createEffect)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Possibly related issues
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 0/10 reviews remaining, refill in 58 minutes and 6 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/components/dialog-release-notes.tsx`:
- Around line 77-79: The description paragraph in DialogReleaseNotes (the <p>
that renders {feature()?.description ?? ""}) is scrollable but not
keyboard-focusable; make it focusable by adding a ref (e.g., descriptionRef) and
a tabindex="0" plus an accessible name (aria-label or role="region" with
aria-label/aria-labelledby) so keyboard users can tab into and scroll it, and
optionally in the component's useEffect check if
descriptionRef.current.scrollHeight > descriptionRef.current.clientHeight and
call descriptionRef.current.focus() to move initial focus when content
overflows; update the element rendering the feature()?.description and the
component lifecycle logic (useEffect) accordingly.
In `@packages/app/src/context/highlights.tsx`:
- Line 12: sliceHighlights currently enforces MAX_RELEASE_HIGHLIGHTS (const
MAX_RELEASE_HIGHLIGHTS = 5) for all release shapes and thus truncates
structured-schema releases that have highlights[].items[]; change
sliceHighlights so the 5-page cap is only applied to the parsed-markdown code
path (or introduce a separate constant like MAX_PARSED_MARKDOWN_HIGHLIGHTS = 5)
and skip slicing for structured-schema releases that use nested highlights
(detect by the presence of highlights[].items or a parsed/structured flag on the
release). Update sliceHighlights to branch on the release shape (e.g.,
release.highlights.some(h => h.items) or release.isParsedMarkdown) and only
apply MAX_PARSED_MARKDOWN_HIGHLIGHTS in the parsed-markdown branch while leaving
the structured-schema branch untrimmed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4bcb63d5-87c9-4cc1-a9d2-787ef946bb42
📒 Files selected for processing (3)
packages/app/src/components/dialog-release-notes.tsxpackages/app/src/context/highlights.test.tspackages/app/src/context/highlights.tsx
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the release notes logic to group multiple bullet points from a single release into a single scrollable view instead of separate pages. The UI is updated to support scrolling for long descriptions, and the parsing logic now formats bullets with a '•' prefix. Feedback suggests that reducing MAX_RELEASE_HIGHLIGHTS to 5 may cause unintended truncation for structured highlights that contain many items; it is recommended to limit the number of versions displayed instead of the total number of items.
- Add source field to ParsedRelease ('release-body' | 'structured')
- Rename MAX_RELEASE_HIGHLIGHTS to MAX_RELEASE_VERSION_PAGES (5)
- Add MAX_STRUCTURED_HIGHLIGHTS (15) for structured schema path
- Refactor sliceHighlights to apply caps per source, not globally
- Rename parseNoticeDescriptions to parseNoticeContent
- Add keyboard accessibility to scrollable description (tabIndex, role, aria-labelledby)
- Add tests: structured 6+ items independent cap, structured 16 items limit
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/app/src/components/dialog-release-notes.tsx (1)
31-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset the description scroll position on page changes.
This scrollable
<p>keeps itsscrollTopwhile only the text content changes, so moving from a long note to the next page can leave the next release opened partway down. Reset the region to the top wheneverindex()changes so each page starts at the beginning.Also applies to: 57-59, 79-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/components/dialog-release-notes.tsx` around lines 31 - 34, The scrollable description element retains its scrollTop across pages; add a ref (e.g., descriptionRef) to the scrollable <p> and reset its scrollTop to 0 whenever the page index changes—either by calling descriptionRef.current.scrollTop = 0 immediately after setIndex(index() + 1) in handleNext (and similar handlers around lines 57-59 and 79-86) or, better, by adding a useEffect that watches index() and sets descriptionRef.current.scrollTop = 0 when index changes; reference setIndex, index(), handleNext and the scrollable paragraph element when making the change.packages/app/src/context/highlights.tsx (1)
109-143:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve prose when a notice mixes paragraphs and bullets.
parseNoticeContent()now only returns eithersummaryorbullets, so any standalone text before the first list item is dropped, and trailing prose gets merged into the last bullet. A notice likeImportant migration notefollowed by bullets will silently lose that intro on the rendered version page.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/context/highlights.tsx` around lines 109 - 143, parseNoticeContent currently discards or merges standalone prose around lists; update parseNoticeContent to detect and preserve leading and trailing non-list paragraphs instead of merging them into the first/last bullet. Specifically, while iterating lines in parseNoticeContent, collect any consecutive non-list lines before the first matched list item as an "intro" (trimNoticeItem applied), collect list items into bullets as now, and collect any non-list lines after the last list item as an "outro"; then return a structure that preserves all parts (e.g., change the returned shape for mixed content to include intro?: string, items: string[], outro?: string or a new kind "mixed") so callers can render intro, bullet list, and outro separately; update references to parseNoticeContent, trimNoticeItem and the result shape accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/app/src/components/dialog-release-notes.tsx`:
- Around line 31-34: The scrollable description element retains its scrollTop
across pages; add a ref (e.g., descriptionRef) to the scrollable <p> and reset
its scrollTop to 0 whenever the page index changes—either by calling
descriptionRef.current.scrollTop = 0 immediately after setIndex(index() + 1) in
handleNext (and similar handlers around lines 57-59 and 79-86) or, better, by
adding a useEffect that watches index() and sets
descriptionRef.current.scrollTop = 0 when index changes; reference setIndex,
index(), handleNext and the scrollable paragraph element when making the change.
In `@packages/app/src/context/highlights.tsx`:
- Around line 109-143: parseNoticeContent currently discards or merges
standalone prose around lists; update parseNoticeContent to detect and preserve
leading and trailing non-list paragraphs instead of merging them into the
first/last bullet. Specifically, while iterating lines in parseNoticeContent,
collect any consecutive non-list lines before the first matched list item as an
"intro" (trimNoticeItem applied), collect list items into bullets as now, and
collect any non-list lines after the last list item as an "outro"; then return a
structure that preserves all parts (e.g., change the returned shape for mixed
content to include intro?: string, items: string[], outro?: string or a new kind
"mixed") so callers can render intro, bullet list, and outro separately; update
references to parseNoticeContent, trimNoticeItem and the result shape
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d84025ea-dbfd-4dff-8491-7904d49df614
📒 Files selected for processing (3)
packages/app/src/components/dialog-release-notes.tsxpackages/app/src/context/highlights.test.tspackages/app/src/context/highlights.tsx
- Add createEffect to reset description scrollTop on page change - Preserve intro prose before bullets in mixed content notices - Add intro field to ParsedNotice bullets kind - Update formatReleaseNoticeDescription to include intro before bullets - Add test for mixed content (intro + bullets) preservation - Update localized update notice test to expect preserved intro
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/context/highlights.tsx`:
- Around line 114-140: The parser currently removes blank lines early which
causes paragraphs after the last list item to be folded into currentBullet;
update the splitting logic so blank lines are preserved (remove the .filter that
drops empty lines) and in the loop treat an empty trimmed line as a paragraph
break: if line.trim() === "" and currentBullet is set, push
trimNoticeItem(currentBullet), set currentBullet = undefined and hasSeenBullet =
true (skip adding to prose); keep the existing rule that prose only accumulates
when !hasSeenBullet, and continue to push any final currentBullet at the end as
before; reference symbols: notice, lines, currentBullet, hasSeenBullet,
trimNoticeItem, bullets, prose.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d1340949-49aa-4e3f-9648-2714eabc982d
📒 Files selected for processing (3)
packages/app/src/components/dialog-release-notes.tsxpackages/app/src/context/highlights.test.tspackages/app/src/context/highlights.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/app/src/context/highlights.test.ts
Ensure DOM is fully updated before resetting scroll position when page index changes.
- Stop filtering empty lines early; use them as paragraph breaks - Flush currentBullet when encountering an empty line - Add test ensuring trailing prose is not folded into final bullet
Owner
Author
|
@gemini-code-assist Thanks for the review. This issue has been addressed in commit e6378dc. Changes made:
|
|
Thanks for the update, @Astro-Han. The changes to separate the pagination caps for release-body and structured highlights, along with the added regression tests, look correct and address the concerns raised. This approach ensures that the UI remains consistent while maintaining the intended data boundaries. |
Astro-Han
force-pushed
the
pawwork/fix-release-notes-pagination
branch
2 times, most recently
from
May 3, 2026 04:39
f840915 to
f095785
Compare
11 tasks
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
Fixes the release notes pagination regression where each bullet was rendered as a separate toast page. Now each release version maps to exactly one page, with all bullets merged into a scrollable description.
Why
Issue #398: PR #372 introduced a behavior regression where
DialogReleaseNotespaginated by individual bullet items instead of by version. A release with 7 bullets would show 7 toast pages. The intended behavior is one page per version.Related Issue
Closes #398
Human Review Status
Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.
Review Focus
parseReleasebody path now returns exactly oneHighlightper release version, while the structuredhighlightsschema path remains unchanged with its own cap.sliceHighlightsnow tracksreleaseBodyPagesandstructuredHighlightsseparately. Release body is capped at 5 version pages; structured schema keeps its 15-item cap.DialogReleaseNotesis independently scrollable withwhitespace-pre-linepreserving bullet line breaks. Title and buttons stay fixed. AddedtabIndex={0},role="region", andaria-labelledbyfor keyboard accessibility.Risk Notes
•prefix due to theParsedNoticediscriminated union.highlightsschema behavior is preserved; a regression test ensures 6+ items are not truncated by the 5-page release-body cap.How To Verify
From repo root:
bun --cwd packages/app test --preload ./happydom.ts ./src/context/highlights.test.ts bun --cwd packages/app typecheckManual UI Check
•prefix and line breaks.Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
Style
Refactor