Skip to content

Feat/sign in mobile#209

Merged
manueltorres0 merged 7 commits intomainfrom
feat/sign-in-mobile
Mar 25, 2026
Merged

Feat/sign in mobile#209
manueltorres0 merged 7 commits intomainfrom
feat/sign-in-mobile

Conversation

@manueltorres0
Copy link
Contributor

@manueltorres0 manueltorres0 commented Mar 24, 2026

Description

DEMO

  • added sign in / sign out / sign up screens and flows for mobile

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (code improvement without changing functionality)
  • Documentation update
  • Configuration/infrastructure change
  • Performance improvement
  • Test coverage improvement

Testing & Validation

How this was tested

  1. Tested all flows via UI

Unfinished Work & Known Issues

  • The following items are intentionally deferred:
    • *** OAuth support to allow people who signed up on web with oauth to use that same account on mobile

Pre-Merge Checklist

Code Quality

  • Code follows the project's style guidelines and conventions
  • Self-review completed (I've reviewed my own code for obvious issues)
  • No debugging code, console logs, or commented-out code left behind
  • No merge conflicts with the base branch
  • Meaningful commit messages that explain the "why"

Testing & CI

  • All CI checks are passing
  • All new and existing tests pass locally
  • Test coverage hasn't decreased (or decrease is justified)
  • Linting passes without errors

Documentation

  • Code is self-documenting or includes helpful comments for complex logic
  • API documentation updated (if backend endpoints changed)
  • Type definitions are accurate and up-to-date

@codecov
Copy link

codecov bot commented Mar 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.00%. Comparing base (eaa77fe) to head (d196fea).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #209       +/-   ##
===========================================
+ Coverage   24.73%   84.00%   +59.26%     
===========================================
  Files          37        7       -30     
  Lines        1237       50     -1187     
  Branches        0       24       +24     
===========================================
- Hits          306       42      -264     
+ Misses        926        8      -918     
+ Partials        5        0        -5     
Flag Coverage Δ
backend ?
mobile 84.00% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 44 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@Dao-Ho Dao-Ho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some small cmts

const [email, setEmail] = useState<string>("");
const [password, setPassword] = useState<string>("");
const [error, setError] = useState("");
const run = useClerkErrorHandler(setError);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make this var more descriptive?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

run(async () => {
if (!isLoaded || !signIn || !setActive) return;
const result = await signIn.create({ identifier: email, password });
if (result.status === "complete") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make complete an enum?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


{error ? (
<Text className="text-danger text-sm mb-4">{error}</Text>
) : null}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
) : null}
{error && (
<Text className="text-danger text-sm mb-4">{error}</Text>
)}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

try {
await action();
} catch (err: any) {
setError(err.errors?.[0]?.message);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good but just wanna double check that this is standard error format for all errors returned by clerk

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, just double checked


{error ? (
<Text className="text-danger text-sm mb-4">{error}</Text>
) : null}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same cmt as above


{error ? (
<Text className="text-danger text-sm mb-4">{error}</Text>
) : null}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same cmt

Copy link
Contributor

@Dao-Ho Dao-Ho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✅ ✅

@manueltorres0 manueltorres0 merged commit bacf1fe into main Mar 25, 2026
7 checks passed
@manueltorres0 manueltorres0 deleted the feat/sign-in-mobile branch March 25, 2026 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants