Fix redundant Provider.of call causing unnecessary rebuilds in AuthScreen#187
Open
AnasNasr-afk wants to merge 2 commits intoAOSSIE-Org:mainfrom
Open
Fix redundant Provider.of call causing unnecessary rebuilds in AuthScreen#187AnasNasr-afk wants to merge 2 commits intoAOSSIE-Org:mainfrom
AnasNasr-afk wants to merge 2 commits intoAOSSIE-Org:mainfrom
Conversation
…vigation in AuthScreen
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds IntelliJ IDEA project/workspace configuration files and refactors auth navigation in Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.idea/vcs.xml (1)
1-6: IDE configuration files should not be committed to version control.The
.idea/directory contains IDE-specific settings that vary per developer. Add.idea/to.gitignoreto prevent these files from being tracked. This applies to all.idea/*files in this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.idea/vcs.xml around lines 1 - 6, Remove IDE-specific files from the repository and stop tracking them by adding ".idea/" to .gitignore and removing the current tracked .idea files from Git; specifically, add the pattern ".idea/" to the .gitignore file, then run git rm --cached on the tracked .idea files (e.g., the VcsDirectoryMappings entry shown in .idea/vcs.xml) and commit that change so the IDE files are no longer tracked going forward..idea/other.xml (1)
1-1319: Remove developer-specific device configuration from version control.This file contains 1300+ lines of device selection preferences that are specific to your local IDE setup. It adds unnecessary repository bloat and will cause merge conflicts when multiple developers work on the project.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.idea/other.xml around lines 1 - 1319, The .idea/other.xml contains developer-specific deviceSelectionList entries (PersistentDeviceSelectionData / component name="direct_access_persist.xml") that should not be versioned; remove this file from the repo, add .idea/other.xml (or the specific .idea pattern that contains direct_access_persist.xml) to .gitignore, run git rm --cached .idea/other.xml and commit the removal, and update the PR to only include a brief README or contributing note explaining how devs can export/import their local device profiles (or use IDE's Settings Repository) instead of committing these PersistentDeviceSelectionData entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.idea/workspace.xml:
- Around line 29-42: The workspace file .idea/workspace.xml contains a local
absolute path in the key "last_opened_file_path" (value
"/Users/anasnasr/StudioProjects/NeuroTrack") and must not be committed; remove
that entry or revert the file to a clean, non-personal version, add
.idea/workspace.xml to .gitignore (or update your repo-level ignore rules) so
workspace files are excluded going forward, and amend the commit (or create a
new one) to remove the sensitive path from history (use an interactive amend or
remove+commit and, if necessary, rewrite history with git rebase or
filter-branch/BFG) so the username/local directory is not present in the repo.
In `@patient/lib/presentation/auth/auth_screen.dart`:
- Around line 126-133: The post-frame callback captures the snapshot `status`
leading to stale navigation when AuthProvider updates rapidly; change the
callback to re-read the current status from AuthProvider inside the callback
(e.g., via context.read<AuthProvider>().authNavigationStatus) and then call
_handleNavigation with that fresh value, while keeping the mounted check and the
conditional that prevents scheduling for unknown status; update the
WidgetsBinding.instance.addPostFrameCallback block accordingly so it never uses
the captured `status` variable.
---
Nitpick comments:
In @.idea/other.xml:
- Around line 1-1319: The .idea/other.xml contains developer-specific
deviceSelectionList entries (PersistentDeviceSelectionData / component
name="direct_access_persist.xml") that should not be versioned; remove this file
from the repo, add .idea/other.xml (or the specific .idea pattern that contains
direct_access_persist.xml) to .gitignore, run git rm --cached .idea/other.xml
and commit the removal, and update the PR to only include a brief README or
contributing note explaining how devs can export/import their local device
profiles (or use IDE's Settings Repository) instead of committing these
PersistentDeviceSelectionData entries.
In @.idea/vcs.xml:
- Around line 1-6: Remove IDE-specific files from the repository and stop
tracking them by adding ".idea/" to .gitignore and removing the current tracked
.idea files from Git; specifically, add the pattern ".idea/" to the .gitignore
file, then run git rm --cached on the tracked .idea files (e.g., the
VcsDirectoryMappings entry shown in .idea/vcs.xml) and commit that change so the
IDE files are no longer tracked going forward.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d9fbc17-d119-41ff-ba0f-97a839a71727
⛔ Files ignored due to path filters (1)
patient/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.idea/NeuroTrack.iml.idea/misc.xml.idea/modules.xml.idea/other.xml.idea/vcs.xml.idea/workspace.xmlpatient/lib/presentation/auth/auth_screen.dart
Comment on lines
+29
to
+42
| <component name="PropertiesComponent"><![CDATA[{ | ||
| "keyToString": { | ||
| "RunOnceActivity.ShowReadmeOnStart": "true", | ||
| "RunOnceActivity.cidr.known.project.marker": "true", | ||
| "RunOnceActivity.readMode.enableVisualFormatting": "true", | ||
| "cf.first.check.clang-format": "false", | ||
| "cidr.known.project.marker": "true", | ||
| "dart.analysis.tool.window.visible": "false", | ||
| "git-widget-placeholder": "main", | ||
| "kotlin-language-version-configured": "true", | ||
| "last_opened_file_path": "/Users/anasnasr/StudioProjects/NeuroTrack", | ||
| "show.migrate.to.gradle.popup": "false" | ||
| } | ||
| }]]></component> |
There was a problem hiding this comment.
Workspace file contains local machine paths and should not be committed.
Line 39 exposes the local path /Users/anasnasr/StudioProjects/NeuroTrack, which reveals your username and local directory structure. Workspace files are inherently per-developer and should be excluded via .gitignore.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.idea/workspace.xml around lines 29 - 42, The workspace file
.idea/workspace.xml contains a local absolute path in the key
"last_opened_file_path" (value "/Users/anasnasr/StudioProjects/NeuroTrack") and
must not be committed; remove that entry or revert the file to a clean,
non-personal version, add .idea/workspace.xml to .gitignore (or update your
repo-level ignore rules) so workspace files are excluded going forward, and
amend the commit (or create a new one) to remove the sensitive path from history
(use an interactive amend or remove+commit and, if necessary, rewrite history
with git rebase or filter-branch/BFG) so the username/local directory is not
present in the repo.
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.
Closes #186
Problem
AuthScreen.build()contained a discardedProvider.of<AuthProvider>call:Provider.of(context, listen: true).authNavigationStatus;
The returned value was never used, but the call still subscribed the widget to
AuthProviderupdates. This caused unnecessary widget rebuilds whenever theprovider changed, creating avoidable UI work.
Additionally:
hasNavigatedflag was present.Changes
Provider.of<AuthProvider>call.hasNavigatedvariable._handleNavigation()method.mountedsafety check before executing navigation from async callbacks.Result
AuthScreen.Files Changed
lib/presentation/auth/auth_screen.dart
Summary by CodeRabbit