-
Notifications
You must be signed in to change notification settings - Fork 57
feat(ui): replace ScrollView with custom VerticalScrollView and update visibility modifiers #195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…e visibility modifiers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a custom VerticalScrollView component by making it public and replaces SwiftUI's native ScrollView with this custom implementation in the sidebar container. The changes also include animation improvements for page transitions.
- Made
VerticalScrollViewstruct and its key methods (init,makeNSView,updateNSView) public to enable use across module boundaries - Replaced
ScrollViewwithVerticalScrollViewinContainerViewfor the tabs list section - Added explicit animation configuration to
NSPageViewfor smoother page transitions with 0.1s duration and ease-out timing
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ora/UI/VerticalScrollView.swift | Added public access modifiers to VerticalScrollView struct and its NSViewRepresentable protocol methods to make the component available for use outside its defining module |
| ora/Modules/Sidebar/ContainerView.swift | Replaced SwiftUI's ScrollView with the custom VerticalScrollView implementation for the tabs list section |
| ora/UI/NSPageView.swift | Added QuartzCore import and implemented explicit animation configuration (duration and timing function) for smoother page index transitions |
| ora/UI/Toast/ToastModel.swift | Added empty file (appears to be accidental or incomplete) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
ora/UI/Toast/ToastModel.swift
Outdated
| @@ -0,0 +1 @@ | |||
|
|
|||
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file appears to be empty (contains only a blank line). If this file is not needed, it should be removed from the commit. If it's a placeholder for future implementation, consider adding a comment explaining its purpose or implement the actual model structure.
| // Placeholder for ToastModel. Implementation will be added in the future. |
idk where this came from 🤷🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| override func animation(forKey key: NSAnimatablePropertyKey) -> Any? { | ||
| if key == "selectedIndex" { | ||
| let animation = CABasicAnimation() | ||
| animation.duration = 0.1 |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The animation duration 0.1 is hardcoded in two places (here and in updateSelectedIndex at line 173). Consider extracting this to a constant to maintain consistency and make future adjustments easier. For example: private static let transitionDuration: TimeInterval = 0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
Pull Request – Ora Browser
Thank you for contributing to Ora. Please fill out this template to help us review your PR efficiently.
Summary
Fixes an issue with swiping between spaces. Before, there was a bug where swiping wouldn't work unless you did it in the favorited tabs region. Now, that swipe-able region has been significantly expanded to the entire sidebar
Type of Change
Select all that apply:
Screenshots / Recordings (if applicable)
Attach screenshots or recordings if your changes modify the UI or user experience.
Screen.Recording.2025-11-28.at.12.14.44.AM.mov
AI Assistance Disclosure
If AI assistance was used for any part of this PR (code, docs, commit messages, or descriptions), please disclose it here:
I used Gemini 3 Pro in GitHub Copilot to identify the files where the VerticalScrollView were defined and I used it on the swiping functionality.
Checklist
Before submitting, ensure the following:
• Code builds successfully on macOS 14+
• All tests pass locally
• Code follows Ora’s formatting and linting standards
• No new warnings or errors
• Descriptive title and summary provided
• Screenshots attached if applicable
• Related issues linked below
Related Issues
Closes #
Additional Notes
Add any relevant context, trade-offs, or future follow-up plans here.