fix(update-modal): scroll long release notes inside the modal - #1807
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughThe update modal can shrink within its bounded height. Its release-notes region fills the available space without the previous maximum-height constraint. An end-to-end test checks scrolling and footer placement with 300 release-note paragraphs. ChangesUpdate modal layout
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue was identified in the modal layout change. Normal checks can proceed before merging. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. A rabbit checks the notes that flow Comment |
The release notes' max-height: 70% never resolved because the modal content has no definite height, so the notes grew the content instead of scrolling. With #1803 showing up to 100 releases, an upgrade from an old version pushed the Done footer ~122,000px down. Let the content shrink to the modal's bounded height and have the notes fill the remaining space so they scroll.
0a0ebbe to
26478b8
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
1 participant
The What's new modal's release notes now scroll inside the modal, so the Done footer stays in view however many releases it lists.
.quickadd-update-modalhadmax-height: 70%, but its parent (the modal's content element,.quickadd-update-modal-container) has no definite height, so the limit never resolved and the notes grew the content instead of scrolling. That went unnoticed while the modal showed one or two releases. Since #1803 it can list up to 100: upgrading from 1.18.1 made the notes ~122,000px tall and put Done that far down. On desktop the X and Esc still close it. On phones Done is the reliable exit (#635), so an old-version upgrader would have to scroll through every release to leave.The fix lets the content element shrink to the modal's bounded height (
min-height: 0) and has the notes fill the remaining space (flex: 1 1 auto) as the scroll region.Verified in Obsidian 1.13.7 with a real 1.18.1 -> master upgrade (live GitHub release notes):
is-phone): same, Done above the home-indicator inset.tests/e2e/update-modal-layout.test.tsbuilds UpdateModal's DOM inside Obsidian's real modal styles and asserts the notes scroll and the footer stays inside the modal. It fails on master and passes with this change.No release or migration impact beyond the CSS. The 2.28.0 release PR (#1790) will need regenerating after this merges.
Note
Fix update modal to scroll long release notes instead of overflowing
Fixes CSS sizing in styles.css so long release notes scroll inside the update modal and the footer stays in view. The modal content container gets a zero minimum height so it can shrink, and the percentage-based max height is replaced with flexible sizing while keeping the scrollable overflow.
Macroscope summarized 26478b8.
Summary by CodeRabbit