Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added warning note to not add WebAuthn #10231

Merged
merged 4 commits into from May 12, 2024
Merged

Conversation

pantheredeye
Copy link
Collaborator

I added a warning note below yarn rw g dbAuth alerting the user to not add WebAuthn. Adding it breaks the login pages and the tutorial does not address how to correct it. Not adding webauthn is the easy answer.

@cannikin What do you think about this language? I wanted to add something like 'feel free to add later' but that just got long. I wanted the warning instead of tip or info because it does break the app if added.

Background: We had a user following the tutorial, added dbAuth with WebAuthn, and it broke her app. She tried debugging and couldn't make headway and finally came to the discord for help. Basically, I deleted the dbauth generated pages, installed it without webauthn and everything worked.

I added a warning note below `yarn rw g dbAuth` alerting the user to not add WebAuthn. Adding it breaks the login pages and the tutorial does not address how to correct it. Not adding webauthn is the easy answer.

@cannikin What do you think about this language? I wanted to add something like 'feel free to add later' but that just got long. I wanted the warning instead of tip or info because it does break the app if added.

Background: We had a user following the tutorial, added dbAuth with WebAuthn, and it broke her app. She tried debugging and couldn't make headway and finally came to the discord for help. Basically, I deleted the dbauth generated pages, installed it without webauthn and everything worked.
@cannikin
Copy link
Member

Ahhh great call, I forgot about the fact that we added WebAuthn support after the tutorial, but never went back and updated it. Thanks! You might want to backport this language into previous doc releases as well. Not sure what Redwood release we added WebAuthn, but probably at least v4? Take a look in the versioned_docs directory were all the previous releases are stored.

@pantheredeye
Copy link
Collaborator Author

Looks like v3 from the release notes.

Do I need to add to v7.0 as well? I'm not sure if what I added to is 7.0 or 7.1 or both.

@cannikin
Copy link
Member

If there's a 7.0 in versioned docs I think you'd need to add it to that, yeah. When you add to just the root it gets assigned to the latest point release, so that's 7.1. You'd need to manually add to 7.0 and below in versioned_docs

@@ -473,6 +473,11 @@ Yet another generator is here for you, this time one that will create pages for
```bash
yarn rw g dbAuth
```
:::warning

In the dbAuth setup steps you will see the option for "Querying WebAuthn addition..." For now, either press enter to keep the default as 'false' or type 'no.' WebAuthn works, and is great, but we do not want to add it as part of the tutorial.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In the dbAuth setup steps you will see the option for "Querying WebAuthn addition..." For now, either press enter to keep the default as 'false' or type 'no.' WebAuthn works, and is great, but we do not want to add it as part of the tutorial.
In the dbAuth setup steps you will see the option for "Querying WebAuthn addition..." For now, either press enter to keep the default as 'false' or type 'no'. WebAuthn works, and is great, but we do not want to add it as part of the tutorial.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Tobbe FWIW, I usually add end of sentence punctuation inside of 'quotes' but outside of codeblocks that land at the end of sentences. I have thought about this in different ways, though. It looks like with American english punctuation lands inside the quotes, but with British english there are cases where this goes either way! I think I like this British rule better:

In British English, periods and commas only go inside quotation marks if the original quoted material also had those punctuation marks.

Copy link
Member

Choose a reason for hiding this comment

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

Ohh, had no idea! That looks so weird to me 😆

In Swedish the rule is: If the sentence starts outside the quotation marks it also ends outside the quotation marks (so the period that ends the sentence goes outside the quotations). If the sentence starts within the quotation marks, it also ends inside the quotation marks (so the period, exclamation mark, etc goes inside the quotation marks).

Copy link
Member

@Tobbe Tobbe left a comment

Choose a reason for hiding this comment

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

Tiny change suggested, but other than that it sounds good to me 👍

@thedavidprice
Copy link
Contributor

@pantheredeye Once the language is finalized, could you move the file changes from the other PRs into this PR, so there's just one PR associated with this change across all doc versions? Lastly, I don't think it's necessary to go back through all doc versions. >= v6 seems reasonable to me. Your call.

Thanks for working on this!

@pantheredeye
Copy link
Collaborator Author

I updated this PR with a commit that

  1. makes the requested punctuation change
  2. updates current, v7.0 and v6.x doc sets

We should be good to go. Please let me know if something isn't working right!

@thedavidprice
Copy link
Contributor

thedavidprice commented May 11, 2024

Thanks @pantheredeye LGTM but I'll assume @Tobbe is managing final review and merge (just let me know otherwise).

I've closed out:

@thedavidprice thedavidprice reopened this May 11, 2024
@Tobbe Tobbe added the release:docs This PR only updates docs label May 12, 2024
@Tobbe Tobbe added this to the chore milestone May 12, 2024
@Tobbe Tobbe merged commit 68f5ccd into redwoodjs:main May 12, 2024
45 checks passed
@pantheredeye pantheredeye deleted the patch-2 branch May 12, 2024 11:17
dac09 added a commit that referenced this pull request May 15, 2024
…uth-store

* 'main' of github.com:redwoodjs/redwood:
  chore(router): Add more code comments to analyzeRoutes (#10569)
  RSC: No need to use memo or useMemo in the server router (#10568)
  RSC: getViteConfig in rscWorker (#10567)
  Split RSC and RSA handling in rscWorker (#10565)
  RSC: Extract the fetchRSC function (#10564)
  RSC: Fix TODO regarding SSR in client.ts (#10562)
  Docs: QoL Updates to Contributing.md (#10561)
  Added warning note to not add WebAuthn (#10231)
  fix(rsc): Load all css links to support css with rsc (#10544)
  fix(cli): Add deprecation notice for edgio deployment (#10551)
  Define `process.env.NODE_ENV` in build process. (#10553)
  chore(docs): Update dbAuth and Supabase middleware READMEs (#10552)
  chore(deps): Upgrade React 19 to beta 20240508 (#10560)
  Revert "chore(deps): React beta 20240508 (#10558)" (#10559)
  chore(deps): React beta 20240508 (#10558)
  fix(functions): Fix context variable warning/error (#10556)
  fix(functions): Mock context in function test template (#10555)
  chore(middleware): Format code and comments and fix comment grammar (#10554)
Josh-Walker-GM pushed a commit that referenced this pull request May 15, 2024
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:docs This PR only updates docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants