-
Notifications
You must be signed in to change notification settings - Fork 131
I have introduced an icon to open circuit link in browser #326
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
I have introduced an icon to open circuit link in browser #326
Conversation
WalkthroughThe changes introduce a new method Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/ui/views/projects/project_details_view.dart (2)
49-59: URL launching implementation looks good, but there's duplicate URL construction logicThe URL launching implementation correctly handles errors and displays appropriate messages to the user.
There's duplicate URL construction logic between this method and
onShareButtonPressed(). Consider extracting this into a helper method:+ String _getProjectUrl() { + return 'https://circuitverse.org/users/${_receivedProject.relationships.author.data.id}/projects/${_receivedProject.id}'; + } Future<void> _launchProjectUrl() async { - final url = - 'https://circuitverse.org/users/${_receivedProject.relationships.author.data.id}/projects/${_receivedProject.id}'; + final url = _getProjectUrl(); final Uri uri = Uri.parse(url); if (!await launchUrl(uri, mode: LaunchMode.externalApplication)) { ScaffoldMessenger.of( context, ).showSnackBar(const SnackBar(content: Text('Could not launch URL'))); } }Then update
onShareButtonPressed()as well:void onShareButtonPressed() { final RenderBox? box = context.findRenderObject() as RenderBox?; - var URL = - 'https://circuitverse.org/users/${widget.project.relationships.author.data.id}/projects/${widget.project.id}'; + var URL = _getProjectUrl(); Share.share( URL, sharePositionOrigin: box!.localToGlobal(Offset.zero) & box.size, ); }
61-69: Inconsistent variable usage in onShareButtonPressed methodThis method still uses
widget.projectdirectly while the rest of the code has been updated to use_receivedProject.Update to use the renamed variable for consistency:
void onShareButtonPressed() { final RenderBox? box = context.findRenderObject() as RenderBox?; var URL = - 'https://circuitverse.org/users/${widget.project.relationships.author.data.id}/projects/${widget.project.id}'; + 'https://circuitverse.org/users/${_receivedProject.relationships.author.data.id}/projects/${_receivedProject.id}'; Share.share( URL, sharePositionOrigin: box!.localToGlobal(Offset.zero) & box.size, ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ui/views/projects/project_details_view.dart(18 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (macos-latest)
🔇 Additional comments (5)
lib/ui/views/projects/project_details_view.dart (5)
22-22: Import added correctly for URL launching functionalityThe
url_launcherpackage is appropriately imported to enable the new feature for opening project links in a browser.
39-39: Variable naming correctionGood fix for the spelling error by renaming
_recievedProjectto_receivedProjectthroughout the code.Also applies to: 46-47
82-91: Browser button implementation is clean and consistentThe browser button follows the same design pattern as the share button, maintaining UI consistency.
622-629: AppBar title truncation fix is appropriateUsing a
SizedBoxwith a constrained width andTextOverflow.ellipsiseffectively handles long titles.
630-633: Action buttons are well-organizedThe browser button and share button are appropriately placed in the app bar actions.
Pull Request Test Coverage Report for Build 14274148299Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
Hey @JatsuAkaYashvant , just check once the comment by codeRabbit above, this is an important improvement by using a helper method for storing url. Will merge once you make these changes. |
…hareButtonPressed()
…hareButtonPressed()
|
@hardik17771 I have resolved all the issues, please review. |
|
🎉 This PR is included in version 1.1.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Fixes #
I have introduced an icon to open circuit link in the browser in one click, resolving the issue : #261
Describe the changes you have made in this PR -
Changes Made
Added Browser Button:
Fixed AppBar Title Truncation:
Code Quality Improvements:
Maintained Existing Functionality:
Screenshots of the changes (If any) -
Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit
New Features
Style
Bug Fixes