Skip to content

Conversation

sofietoft
Copy link
Contributor

Description

Additional configuration is required for using External Login Providers on Cloud.
I added this information in a note.

Type of suggestion

  • Typo/grammar fix
  • Updated outdated content
  • New content
  • Updates related to a new version
  • Other

Copy link
Contributor

@eshanrnh eshanrnh left a comment

Choose a reason for hiding this comment

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

Looks good, @sofietoft 🙌 I have a few minor suggestions.

However, I think the Note might be a bit too long.

How about keeping just the first two sentences in the Note, and moving the rest of the information/steps below it?

Alternatively, you could add it as a sub-section, which would create a heading in the right navigation and make it easier to find.

@sofietoft
Copy link
Contributor Author

Thanks for the review @eshanrnh - So, a bit of "history" here:
I started by adding this as a note through GitBook, but since I couldn't add lists or code snippets into the note, I changed it to a section instead.
@jonat123 reviewed it, and suggested a note instead, which made me make the PR through GitBook so that I could add steps and the code snippets to the note 😂

I'm not a fan of the formatting of the text within the note, so I'm happy with changing it back!

@eshanrnh
Copy link
Contributor

eshanrnh commented Sep 6, 2024

Ah, I see... 🙈 It might be a good idea to shorten the Note, or maybe use expandable sections? Right now, the lists and code snippets are making it feel a bit overwhelming, which takes away from the purpose of highlighting important info in the Note.

I’ll leave it up to you to decide the best formatting for the text 😀

@sofietoft
Copy link
Contributor Author

@eshanrnh - I've taken it out of the note.
Looks a lot cleaner if you ask me.

I'll merge it, if you have no further comments?

Copy link
Contributor

@eshanrnh eshanrnh left a comment

Choose a reason for hiding this comment

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

Looks good. Please feel free to merge it in.

@sofietoft sofietoft merged commit 9f46814 into main Sep 6, 2024
2 of 3 checks passed
@sofietoft sofietoft deleted the ExternalLoginProviderCloud branch September 20, 2024 10:33
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