refactor: login form #24347
refactor: login form #24347
Conversation
alextran1502
force-pushed
the
refactor-login-form
branch
from
a9d54eb to
76f1e0c
Compare
alextran1502
force-pushed
the
refactor-login-form
branch
2 times, most recently
from
bc3a71a to
6b312eb
Compare
alextran1502
force-pushed
the
refactor-login-form
branch
from
6b312eb to
a739be3
Compare
shenlong-tanwen
left a comment
shenlong-tanwen
left a comment
There was a problem hiding this comment.
If we're to start refactoring the widget code, Can we decide on how to want to handle the global / widget states and how we want the flow to be like before refactoring pages?
For instance, we don't really need a notifier just to pass method calls over to the service layer. We also should be having UI components like the ui library that we use as building blocks for the widget. The LoginCredentialsForm still takes in a lot of props as params but it will only be ever used in the login screen.
Ideally, we'd have global providers that persists values for the entirety of the app life cycle and we'd have auto disposable page level providers that maintains all the state logic for the current page. Most of our logic should be moved to such a provider and the Widget class should only be about building widgets. This would also make it easier to mock the provider and test the widget and provider independently
| return ServerAuthSettings( | ||
| endpoint: endpoint, | ||
| isOAuthEnabled: features.oauthEnabled, | ||
| isPasswordLoginEnabled: features.passwordLogin, | ||
| oAuthButtonText: config.oauthButtonText.isNotEmpty ? config.oauthButtonText : 'OAuth', | ||
| ); | ||
| } |
There was a problem hiding this comment.
We have ServerInfoNotifier that fetches and stores all the above params, are we moving away from using it?
| class OAuthNotifier extends StateNotifier<AsyncValue<void>> { | ||
| final OAuthService _oAuthService; | ||
|
|
||
| OAuthNotifier(this._oAuthService) : super(const AsyncValue.data(null)); | ||
|
|
||
| Future<OAuthLoginData?> getOAuthLoginData(String serverUrl) { | ||
| return _oAuthService.getOAuthLoginData(serverUrl); | ||
| } | ||
|
|
||
| Future<LoginResponseDto?> completeOAuthLogin(OAuthLoginData oAuthData) { | ||
| return _oAuthService.completeOAuthLogin(oAuthData); | ||
| } | ||
| } |
There was a problem hiding this comment.
Feels redundant. We can access and use the oAuthServiceProvider provider directly
Uh oh!
There was an error while loading. Please reload this page.