Conversation
|
Thanks @Miduo666. Please fix the conflict and merge the latest dev. Ask copilot to review. |
Hi @gjwgit Also, just a quick heads-up: the code depends on the new solidpod package (which is still in PR). Copilot might throw an error like The named parameter 'wasAlreadyLoggedIn' isn't defined. because it hasn't seen the updated definitions in the pending package PR yet |
|
Also I can not request Copilot in Solidpod and Solid_auth |
There was a problem hiding this comment.
Pull request overview
Optimizes the SolidUI login flow to reduce redundant work during authentication and to avoid blocking the initial UI while login assets/app metadata are being resolved (issue #224).
Changes:
- Passes precomputed login status into
solidAuthenticate()to avoid repeatingisUserLoggedIn()work. - Removes the “wait for assets to resolve” loading gate and triggers a rebuild once higher-quality assets are resolved.
- Parallelizes package info + default folder/file generation and consolidates into a single
setState().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
lib/src/widgets/solid_login_auth_handler.dart |
Avoids redundant login-state work by forwarding wasAlreadyLoggedIn into solidAuthenticate(). |
lib/src/widgets/solid_login.dart |
Improves perceived latency by not blocking first paint, and reduces init time via parallel async work + single rebuild. |
Comments suppressed due to low confidence (1)
lib/src/widgets/solid_login.dart:340
effectiveImagefalls back towidget.imagewhile asset resolution is still in progress. If the caller provides an AssetImage path that is not in the bundle (the case this resolver is designed to handle), Flutter will attempt to load that missing asset during the first frame and emit an image loading exception (and may show a broken image) before_resolvedImageis set. Consider using a known-good placeholder (e.g.,SolidConfig.soliduiDefaultImage) until_resolvedImageis available, or otherwise ensuring the initial image load cannot reference a missing asset (e.g., via an errorBuilder-based Image widget rather than DecorationImage).
// Use resolved image if available, otherwise fall back to the widget's
// own AssetImage immediately — this prevents the UI being blocked by a
// loading spinner while assets are resolving in the background.
final effectiveImage = _resolvedImage ?? widget.image;
// The login box's default image Widget for the left/background panel
// depending on screen width.
final loginBoxDecor = BoxDecoration(
image: DecorationImage(image: effectiveImage, fit: BoxFit.cover),
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Start fetching app info immediately — it is independent of appDirName. | ||
| final appInfoFuture = getAppNameVersion(); | ||
| await setAppDirName(widget.appDirectory); | ||
| final folders = await generateDefaultFolders(); | ||
| final files = await generateDefaultFiles(); | ||
| // Now that appDirName is set, resolve folders/files in parallel with the | ||
| // already-running appInfo future. | ||
| final results = await Future.wait([ | ||
| generateDefaultFolders(), | ||
| generateDefaultFiles(), | ||
| appInfoFuture, | ||
| ]); | ||
| final customFolders = generateCustomFolders(widget.customFolderPathList); | ||
| if (!mounted) return; | ||
| // Single setState: avoids two separate widget rebuilds. | ||
| setState(() { | ||
| defaultFolders = folders + customFolders; | ||
| defaultFiles = files; | ||
| }); | ||
| final appInfo = await getAppNameVersion(); | ||
| if (!mounted) return; | ||
| setState(() { | ||
| defaultFolders = (results[0] as List<String>) + customFolders; | ||
| defaultFiles = results[1] as Map<dynamic, dynamic>; | ||
| final appInfo = results[2] as ({String name, String version}); | ||
| appName = appInfo.name; |
There was a problem hiding this comment.
Using Future.wait([...]) with heterogeneous futures forces the results into a positional List and requires runtime casts by index (results[0] as ...). This is brittle and can silently break if any return type changes. A more maintainable approach is to start the typed futures (final foldersFuture = ..., etc.), await Future.wait<void>([foldersFuture, filesFuture, appInfoFuture]), and then read the typed results from the original futures/variables.
| // Use resolved logo if available, otherwise fall back to widget's own logo. | ||
|
|
||
| final effectiveLogo = _resolvedLogo ?? SolidConfig.soliduiDefaultLogo; | ||
| final effectiveLogo = _resolvedLogo ?? widget.logo; | ||
|
|
There was a problem hiding this comment.
effectiveLogo also falls back to widget.logo before _resolvedLogo is computed. If widget.logo points to a missing asset, the first build can trigger an image loading exception before the resolver falls back to the solidui defaults. Consider using a guaranteed-existing placeholder until _resolvedLogo is set (or ensure missing assets are handled without throwing during the initial frame).
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: