Skip to content
This repository has been archived by the owner on Oct 7, 2022. It is now read-only.

Sign in flow #169

Merged
merged 3 commits into from
Nov 18, 2021
Merged

Sign in flow #169

merged 3 commits into from
Nov 18, 2021

Conversation

Carla-Moz
Copy link
Contributor

@Carla-Moz Carla-Moz commented Nov 16, 2021

This PR includes three changes:

  1. It updates the sign-in page with the new sign-in, create-acct flow
  2. Adds firebase errors to the UI:
    • sign-in with wrong pw
    • signin with invalid email or user not found
    • create account with an email that already exists
  3. Updates the ux tests to according the new flow

closes #158

@Carla-Moz Carla-Moz requested review from rhelmer and stansky November 16, 2021 17:37
@Carla-Moz Carla-Moz self-assigned this Nov 16, 2021
Copy link
Contributor

@rhelmer rhelmer left a comment

Choose a reason for hiding this comment

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

The code lgtm, I left a few comments wondering if we can remove commented-out code vs. leaving it, but if you have plans for it it's fine.

Two things I noticed while testing:

  1. the font on the sidebar looks different, is this intentional?

Screen Shot 2021-11-17 at 8 25 25 AM

2. I'm seeing different variations of this stack trace on startup:
8:24:03 AM [vite] Error when evaluating SSR module /Users/mozilla/src/rally-web-platform/src/routes/__layout.svelte:
TypeError: Line must be greater than or equal to 1, got -1
    at BasicSourceMapConsumer.SourceMapConsumer_findMapping [as _findMapping] (/Users/mozilla/src/rally-web-platform/node_modules/vite/dist/node/chunks/dep-85dbaaa7.js:65402:13)
    at BasicSourceMapConsumer.SourceMapConsumer_originalPositionFor [as originalPositionFor] (/Users/mozilla/src/rally-web-platform/node_modules/vite/dist/node/chunks/dep-85dbaaa7.js:65471:22)
    at /Users/mozilla/src/rally-web-platform/node_modules/vite/dist/node/chunks/dep-85dbaaa7.js:66410:34
    at String.replace (<anonymous>)
    at /Users/mozilla/src/rally-web-platform/node_modules/vite/dist/node/chunks/dep-85dbaaa7.js:66400:21
    at Array.map (<anonymous>)
    at ssrRewriteStacktrace (/Users/mozilla/src/rally-web-platform/node_modules/vite/dist/node/chunks/dep-85dbaaa7.js:66399:10)
    at instantiateModule (/Users/mozilla/src/rally-web-platform/node_modules/vite/dist/node/chunks/dep-85dbaaa7.js:66544:28)

From a quick search it looks like it's something specific to vite's SSR in dev mode, maybe introduced in a recent update? It doesn't seem to be causing test failures and nothing I can find wrong while testing, but it'd be good to understand why it's happening.

btnID = {welcomeCard ? "signin" : "create"}
on:click={() => {
welcomeCard ? handleTrigger("signin") : handleTrigger("create");
console.log("THIS IS A HIT ON LAUNCH CARD")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this just for debugging?

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! I realized after that I forgot to delete it.

@rhelmer
Copy link
Contributor

rhelmer commented Nov 17, 2021

The code lgtm, I left a few comments wondering if we can remove commented-out code vs. leaving it, but if you have plans for it it's fine.

Two things I noticed while testing:

1. the font on the sidebar looks different, is this intentional?

For reference, on staging it looks like:

image

@Carla-Moz
Copy link
Contributor Author

Carla-Moz commented Nov 17, 2021

The code lgtm, I left a few comments wondering if we can remove commented-out code vs. leaving it, but if you have plans for it it's fine.

Two things I noticed while testing:

1. the font on the sidebar looks different, is this intentional?
Screen Shot 2021-11-17 at 8 25 25 AM 2. I'm seeing different variations of this stack trace on startup:
8:24:03 AM [vite] Error when evaluating SSR module /Users/mozilla/src/rally-web-platform/src/routes/__layout.svelte:
TypeError: Line must be greater than or equal to 1, got -1
    at BasicSourceMapConsumer.SourceMapConsumer_findMapping [as _findMapping] (/Users/mozilla/src/rally-web-platform/node_modules/vite/dist/node/chunks/dep-85dbaaa7.js:65402:13)
    at BasicSourceMapConsumer.SourceMapConsumer_originalPositionFor [as originalPositionFor] (/Users/mozilla/src/rally-web-platform/node_modules/vite/dist/node/chunks/dep-85dbaaa7.js:65471:22)
    at /Users/mozilla/src/rally-web-platform/node_modules/vite/dist/node/chunks/dep-85dbaaa7.js:66410:34
    at String.replace (<anonymous>)
    at /Users/mozilla/src/rally-web-platform/node_modules/vite/dist/node/chunks/dep-85dbaaa7.js:66400:21
    at Array.map (<anonymous>)
    at ssrRewriteStacktrace (/Users/mozilla/src/rally-web-platform/node_modules/vite/dist/node/chunks/dep-85dbaaa7.js:66399:10)
    at instantiateModule (/Users/mozilla/src/rally-web-platform/node_modules/vite/dist/node/chunks/dep-85dbaaa7.js:66544:28)

From a quick search it looks like it's something specific to vite's SSR in dev mode, maybe introduced in a recent update? It doesn't seem to be causing test failures and nothing I can find wrong while testing, but it'd be good to understand why it's happening.

This is weird re the sidebar styles cause I don't see it on my local. I'll do another review of the CSS again. Most likely forgot to update some styles or something. Ah, just a browser issue. Firefox v Chrome. Always gotta remember to look at both, :P.

As for the vite error, I'll investigate as well. Don't want it to potentially break something down the line.

@Carla-Moz Carla-Moz requested a review from rhelmer November 17, 2021 17:58
@Carla-Moz Carla-Moz merged commit 28c4fdd into master Nov 18, 2021
@Carla-Moz Carla-Moz deleted the sign-in-flow branch November 18, 2021 15:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Firebase auth errors in UI
3 participants