chore: update the text below CNAME config errors #2458
Conversation
The old text either did not include anything about CAA records or it wasn't clear enough that an "issue" policy for certainly.com is needed.
WalkthroughAdds a new Svelte component src/lib/components/domains/cnameAlert.svelte that renders an inline alert with static guidance, InlineCode snippets, a line break, and a muted external link to Appwrite docs. Integrates this component into src/lib/components/domains/cnameTable.svelte, src/lib/components/domains/recordTable.svelte, and src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/recordsCard.svelte, replacing prior inline alert/typography/link/icon blocks. Updates imports accordingly by removing unused UI elements (e.g., Alert, Link, Icon, IconInfo) and adding the CnameAlert import. Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors CNAME configuration error messaging by creating a reusable CnameAlert component that provides clearer instructions about CAA records and SSL setup requirements.
- Extracts repeated CNAME alert text into a dedicated reusable component
- Improves clarity of CAA record requirements by specifying the need for an "issue" policy for "certainly.com"
- Consolidates inconsistent alert messaging across multiple domain-related components
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/lib/components/domains/cnameAlert.svelte | New component containing improved CNAME alert text with clearer CAA record instructions |
| src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/recordsCard.svelte | Replaced inline alert with CnameAlert component |
| src/lib/components/domains/recordTable.svelte | Replaced inline alert with CnameAlert component |
| src/lib/components/domains/cnameTable.svelte | Replaced inline alert with CnameAlert component |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| If your domain uses CAA records, ensure there is an <InlineCode code="issue" /> policy for <InlineCode | ||
| code="certainly.com" /> - otherwise, SSL setup may fail. | ||
| </p> | ||
| <br /> |
There was a problem hiding this comment.
Using <br /> for spacing is not a best practice. Consider using CSS margins or padding, or wrapping the content in separate elements with appropriate spacing.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
makes sense, @stnguyen90 you can use a Layout.Stack if needed for adding space between contents 👍
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/components/domains/cnameAlert.svelte (1)
6-19: Good consolidation of CNAME guidance.The component successfully addresses the PR objective by explicitly mentioning CAA records and the "issue" policy requirement for certainly.com. The reusable structure eliminates duplication across multiple files.
Optional: For improved accessibility, consider making the link text more descriptive:
- A list of all domain providers and their DNS setting is available <Link + View the <Link variant="muted" external - href="https://appwrite.io/docs/advanced/platform/custom-domains">here</Link + href="https://appwrite.io/docs/advanced/platform/custom-domains">complete list of domain providers and DNS settings</Link >.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/lib/components/domains/cnameAlert.svelte(1 hunks)src/lib/components/domains/cnameTable.svelte(2 hunks)src/lib/components/domains/recordTable.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/recordsCard.svelte(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (7)
src/lib/components/domains/cnameAlert.svelte (1)
1-4: LGTM!The imports are correctly structured and bring in the necessary components for the alert.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/recordsCard.svelte (2)
12-12: LGTM!The CnameAlert import is correctly added and aligns with the component refactoring across the PR.
49-49: LGTM!The CnameAlert component is properly integrated, replacing the previous inline UI block with the centralized component.
src/lib/components/domains/cnameTable.svelte (2)
2-4: LGTM!The import updates correctly reflect the component refactoring: removing now-unused UI elements and adding the CnameAlert component.
48-48: LGTM!The CnameAlert component replaces the previous inline implementation, maintaining consistency with the other domain-related components.
src/lib/components/domains/recordTable.svelte (2)
3-5: LGTM!The import updates correctly remove the Alert component and add the CnameAlert component, reflecting the refactoring changes.
61-71: LGTM!The conditional usage is well-structured: CnameAlert provides specific CAA record guidance for the 'cname' variant, while non-cname variants retain their generic documentation link. This differentiation is appropriate and maintains clarity.
| If your domain uses CAA records, ensure there is an <InlineCode code="issue" /> policy for <InlineCode | ||
| code="certainly.com" /> - otherwise, SSL setup may fail. | ||
| </p> | ||
| <br /> |
There was a problem hiding this comment.
makes sense, @stnguyen90 you can use a Layout.Stack if needed for adding space between contents 👍
| </Table.Cell> | ||
| </Table.Row.Base> | ||
| </Table.Root> | ||
| <Layout.Stack gap="s" direction="row" alignItems="center"> |
There was a problem hiding this comment.
we probably don't need this stack here.
| </Table.Cell> | ||
| </Table.Row.Base> | ||
| </Table.Root> | ||
| <Layout.Stack gap="s" direction="row" alignItems="center"> |
2 participants
What does this PR do?
The old text either did not include anything about CAA records or it wasn't clear enough that an "issue" policy for certainly.com is needed.
Test Plan
Before:
After:
Related PRs and Issues
None
Have you read the Contributing Guidelines on issues?
Yes
Summary by CodeRabbit