-
Notifications
You must be signed in to change notification settings - Fork 1
fix login issue #143
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: dev
Are you sure you want to change the base?
fix login issue #143
Conversation
SummaryThis PR fixes critical login and authentication issues in the solidui package. Key Fixes
|
|
Hi @lambadajester50-byte, could you please describe in brief what issue have you fixed in this PR? Thank you very much. |
Thank you! I‘ve fixed a login bug described in gjwgit/geopod#13 |
|
Hi Tony @tonypioneer , |
Hi @lambadajester50-byte, thanks for the quick reply. I added the depedencies to the override section, and ran
|
Hi @tonypioneer I have now refactored the code, fixed the configuration issue, and pushed the updates to the repository. I’ve tested the entire flow on Web, and everything is now working perfectly on my end. Please pull the latest changes, run flutter pub get again, and give it another try. Let me know if it works for you! |
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.
Thanks @lambadajester50-byte. I did a round of test. The functionality works well now. Please resolve the conflicts against the dev branch, and lint the code. Because in the code style guidelines of the SII, a blank line before and after a comment is recommended. The exception is at the beginning of a block which lint usually enforces (https://survivor.togaware.com/gnulinux/flutter-style-comments.html).
I‘ve done! Thank you Tony! |
|
Hi @Miduo666, I think there are some conflicts in the code with the dev branch. |
Thank you Tony, I 'll merge dev into this PR soon |
tonypioneer
left a comment
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.
Hi @Miduo666, thanks for the fix.
I did a round of test, the functionality works well on my machine. However, when I ran make prep to do the lint check, there were some minor issues as below.
Dart: FIX IMPORT ORDER
❌ Import ordering issues found in lib/src/widgets/solid_status_bar.dart:
• Import 'package:solidpod/solidpod.dart' should come before 'package:gap/gap.dart' (alphabetical order)
• Missing blank lines between different import groups
📝 Summary: Found import ordering issues in 1 file(s).
💡 To fix these issues, run the same command without --set-exit-if-changed
Successfully fixed import ordering in 202 file(s).
------------------------------------------------------------------------
Dart: REVIEW DEPENDENCIES.
dependency_validator
Validating dependencies for solidui...
These packages are used outside lib/ but are not dev_dependencies:
* flutter_test
* myapp
These packages may be unused, or you may be using assets from these packages:
* web
make: [depend] Error 1 (ignored)
Hi Tony, it's fixed now. I've also enabled login support for the example project, so it's all good to go. |
Thanks @Miduo666. Please merge dev to your branch again. There are still some conflicts. |
Done. Thank you Tony! |
Thanks @Miduo666. I think there are still some conflicts. Maybe the dev branch is updated too frequently. |
Yeah it is. Fortunately I ve resolved the conflicts @tonypioneer |
tonypioneer
left a comment
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.
Works well on my machine. Thanks @Miduo666.
| if (!fileName.endsWith('.enc.ttl') && !fileName.endsWith('.ttl')) { | ||
| // Skip ACL files and metadata files, but include all other file types | ||
| // (TTL, JSON, CSV, TXT, etc.) | ||
| if (fileName.endsWith('.acl') || fileName.endsWith('.meta')) { |
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.
Please add an empty line after the comment. Ref: https://survivor.togaware.com/gnulinux/flutter-style-comments.html
| _currentWebId != null && _currentWebId!.isNotEmpty; | ||
|
|
||
| if (isCurrentlyLoggedIn) { | ||
| // Logout scenario - don't recheck status as page will reload |
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.
Please add an empty line after the comment. Ref: https://survivor.togaware.com/gnulinux/flutter-style-comments.html
| SolidAuthHandler.instance.handleLogout(context); | ||
| } | ||
| } else { | ||
| // Login scenario - can delay status check |
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.
Please add an empty line after the comment. Ref: https://survivor.togaware.com/gnulinux/flutter-style-comments.html
| _checkLoginStatus(); | ||
| } | ||
| }); | ||
| // Only refresh status for login scenario |
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.
Please add an empty line after the comment. Ref: https://survivor.togaware.com/gnulinux/flutter-style-comments.html
lib/src/widgets/solid_login.dart
Outdated
| WidgetsBinding.instance.addObserver(this); | ||
| solidThemeNotifier.addListener(_onThemeChanged); | ||
|
|
||
| // Initialize the controller with the widget's webID |
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.
- Please add an empty line after the comment. Ref: https://survivor.togaware.com/gnulinux/flutter-style-comments.html
- Please use British English.
| context, | ||
| // Use simple pushReplacement instead of pushAndRemoveUntil | ||
| // This preserves the navigation history and doesn't destroy all widgets | ||
| await Navigator.of(context).pushReplacement( |
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.
Please add an empty line after the comment. Ref: https://survivor.togaware.com/gnulinux/flutter-style-comments.html
| if (await logoutPod()) { | ||
| // Navigate to login page after successful logout | ||
| // Works consistently across all platforms | ||
| if (context.mounted) { |
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.
Please add an empty line after the comment. Ref: https://survivor.togaware.com/gnulinux/flutter-style-comments.html
| ) async { | ||
| // Import at top: import 'package:solidpod/solidpod.dart' show getWebId; | ||
| // Check if user is logged in first | ||
| try { |
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.
Please add an empty line after the comment. Ref: https://survivor.togaware.com/gnulinux/flutter-style-comments.html







Pull Request Details
What issue does this PR address
Associated Issue
Type of Change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist
Complete the check-list below to ensure your branch is ready for PR.
make preporflutter analyze lib)dart testoutput or screenshot included in issue #Finalising
Once PR discussion is complete and reviewers have approved: