Skip to content

feat: onLogin hook, saved-locale application and five form components - #167

Merged
anilcancakir merged 2 commits into
mainfrom
feature/login-hook-and-components
Sep 25, 2026
Merged

anilcancakir merged 2 commits into
mainfrom
feature/login-hook-and-components

Conversation

@anilcancakir

Copy link
Copy Markdown
Member

feat: add an onLogin hook, apply the user's saved locale, and five form components

MagicStarterManager.onLogin runs after a fresh sign-in (AuthLogin: password,
two-factor, social, guest, OTP), unawaited and logged on failure; it does not
run on a restore. The signed-in user's saved locale is applied on sign-in and
on restore through MagicStarter.manager.applyLocale, one scheduling point with
a pending and an in-flight target, so a profile save and the restore listener
switch once and a target that changes mid-load wins. Gated by
magic_starter.localization.apply_user_locale (default true).

New components: MSFormActions, MSSwitchRow, MSHeaderAction, MSKeyValueEditor
(with MSKeyValueRow) and MSStringValueList. Every visible string is a
parameter. Uses only magic API available on master.

No version bump.

Part of the "framework-level support" round (plan 2 of 3): code that uptizm carried because a plugin lacked a seam moves into the owning package. Companion PRs: fluttersdk/magic, fluttersdk/magic_starter, fluttersdk/magic_notifications, fluttersdk/magic_deeplink, fluttersdk/wind, fluttersdk/magic_sentry, and the uptizm adoption.

This PR compiles and tests against its siblings' current default branches; nothing here depends on another PR in the round being merged first. No version bump and no publish.

…rm components

MagicStarterManager.onLogin runs after a fresh sign-in (AuthLogin: password,
two-factor, social, guest, OTP), unawaited and logged on failure; it does not
run on a restore. The signed-in user's saved locale is applied on sign-in and
on restore through MagicStarter.manager.applyLocale, one scheduling point with
a pending and an in-flight target, so a profile save and the restore listener
switch once and a target that changes mid-load wins. Gated by
magic_starter.localization.apply_user_locale (default true).

New components: MSFormActions, MSSwitchRow, MSHeaderAction, MSKeyValueEditor
(with MSKeyValueRow) and MSStringValueList. Every visible string is a
parameter. Uses only magic API available on master.

No version bump.
@codecov

codecov Bot commented Sep 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kodizm

kodizm Bot commented Sep 25, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

Looks correct and ready to merge. I found one narrow gap in the locale scheduler that can leave a stale pending target behind.

Minor

lib/src/magic_starter_manager.dart:352 (correctness): _pendingLocale is not cleared when the chase is skipped because next == code. Here is how to trigger it:

  1. The locale is en and tr is loading.
  2. applyLocale('en') sets pending to en.
  3. applyLocale('tr') sets pending back to tr. Its callback backs off because _applyingLocale != null.
  4. When tr settles, next == code, so nothing runs and nothing clears _pendingLocale.

The stuck 'tr' then causes a later applyLocale('tr') to return early at line 305 (code == _pendingLocale), even if Lang.current has since moved to another locale by some other route. The fix is to clear _pendingLocale whenever it equals code after the switch settles. A test for the tr→en→tr in-flight sequence would pin this down.

Tests

There are tests for:

  • the onLogin hook and the saved-locale flow (test/providers/login_hook_and_locale_test.dart), covering dedup, supersede-before-frame, reset, and chase-after-in-flight
  • each of the five new components

Not covered: the in-flight sequence where the target is reverted and then restored.

CI

  • Lint & Test: success
  • Published graph: success
  • codecov/patch: success
  • Auto-merge low-risk Dependabot PRs: skipped

Other notes:

  • I read every lib/ change in full. Of the tests, I only read the end of the locale test file; I skimmed the other test files and the docs, CHANGELOG and stub changes, and did not open the preview or recipe files.
  • The profile-save and provider-listener paths both read the same locale attribute that doUpdateProfile persists, so they stay consistent.
  • The new components follow the four-file folder rule in .claude/rules/components.md, extract Icons.* into static consts, and take every visible string as a parameter.

@anilcancakir

Copy link
Copy Markdown
Member Author

@kodizm review. Minor fixed: a pending target equal to the code just applied is now cleared after the switch settles, so a tr, en, tr sequence during a load leaves nothing stale behind (new test, red first).

@kodizm

kodizm Bot commented Sep 25, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The fix is correct and the PR still looks ready to merge. The earlier Minor finding is resolved, and I found nothing new.

Since the last review: there is one new commit (bc04da6). It changes _performLocaleSwitch and adds one test.

What the change does, at lib/src/magic_starter_manager.dart:355:

  • After a switch settles, it now clears _pendingLocale when the pending target equals the code that was just applied.
  • It still chases any different pending target, as before.
  • As a result, the tr → en → tr sequence during a load no longer leaves a stale 'tr'. That stale value is what made the code == _pendingLocale early return at line 305 skip a later real request.

I checked the failure path too. If Lang.setLocale throws, the matching pending target is still cleared. The error is logged, and the next applyLocale for that code can retry instead of being skipped.

Tests

The new test is test/providers/login_hook_and_locale_test.dart: "a target reverted and restored while its switch is in flight leaves nothing pending behind". It covers the exact sequence from the earlier finding:

  1. Hold the tr load open.
  2. Call applyLocale('en'), then applyLocale('tr').
  3. Let the load settle.
  4. Move the app back to en from outside the manager.
  5. Assert that applyLocale('tr') switches again.

CI

  • Lint & Test: success
  • Published graph: success
  • codecov/patch: success
  • Auto-merge low-risk Dependabot PRs: skipped

I read only the new commit this round. The rest of the PR is as covered in my earlier review.

@anilcancakir
anilcancakir merged commit 3ed2984 into main Sep 25, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant