-
Notifications
You must be signed in to change notification settings - Fork 197
feat(navbar): move Docs link to header (appwrite#2541) #2582
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
base: main
Are you sure you want to change the base?
Conversation
Console (appwrite/console)Project ID: Sites (1)
Tip SSR frameworks are fully supported with configurable build runtimes |
WalkthroughA single-file update to the navbar component adds two new UI elements: a desktop-only navigation anchor that links to the current organization and a compact Docs button that opens the Appwrite documentation site in a new tab. The change also includes minor CSS comment removals and whitespace adjustments. No logic modifications or exported entity changes are introduced. Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
appwrite/docker-compose.yml (1)
67-67: Consider documenting optional ClamAV service.The ClamAV antivirus service is commented out in both the dependency list (Line 67) and service definition (Lines 946-953). While the commenting is consistent, consider either:
- Adding a comment explaining why ClamAV is optional and how to enable it
- Removing the commented code entirely if it's not needed
This would improve clarity for operators deploying this stack.
Also applies to: 946-953
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
appwrite/docker-compose.yml(1 hunks)src/lib/components/navbar.svelte(3 hunks)
🔇 Additional comments (3)
src/lib/components/navbar.svelte (2)
126-126: Minor formatting change.This whitespace addition has no functional impact.
199-206: LGTM!The Docs button implementation is clean and follows best practices. The
rel="noreferrer"attribute properly secures the external link, and the compact styling is consistent with other navbar buttons.appwrite/docker-compose.yml (1)
1-973: Inconsistency with PR objectives.This docker-compose.yml file introduces a comprehensive Appwrite deployment infrastructure with Traefik, multiple worker services, databases, and networking configuration. However, the PR objectives state this is about moving the Docs link from footer to header in the navbar component.
This appears to be either:
- Accidentally committed to the wrong PR
- A merge/rebase issue
- Missing context in the PR description
Please clarify whether this file should be part of this PR.
appwrite/docker-compose.yml
Outdated
| - _APP_ENV | ||
| - _APP_WORKER_PER_CORE | ||
| - _APP_LOCALE | ||
| - _APP_COMPRESSION_MIN_SIZE_BYTES | ||
| - _APP_CONSOLE_WHITELIST_ROOT | ||
| - _APP_CONSOLE_WHITELIST_EMAILS | ||
| - _APP_CONSOLE_SESSION_ALERTS | ||
| - _APP_CONSOLE_WHITELIST_IPS | ||
| - _APP_CONSOLE_HOSTNAMES | ||
| - _APP_SYSTEM_EMAIL_NAME | ||
| - _APP_SYSTEM_EMAIL_ADDRESS | ||
| - _APP_EMAIL_SECURITY | ||
| - _APP_SYSTEM_RESPONSE_FORMAT | ||
| - _APP_OPTIONS_ABUSE | ||
| - _APP_OPTIONS_ROUTER_PROTECTION | ||
| - _APP_OPTIONS_FORCE_HTTPS | ||
| - _APP_OPTIONS_ROUTER_FORCE_HTTPS | ||
| - _APP_OPENSSL_KEY_V1 | ||
| - _APP_DOMAIN | ||
| - _APP_DOMAIN_TARGET_CNAME | ||
| - _APP_DOMAIN_TARGET_AAAA | ||
| - _APP_DOMAIN_TARGET_A | ||
| - _APP_DOMAIN_TARGET_CAA | ||
| - _APP_DNS | ||
| - _APP_DOMAIN_FUNCTIONS | ||
| - _APP_REDIS_HOST | ||
| - _APP_REDIS_PORT | ||
| - _APP_REDIS_USER | ||
| - _APP_REDIS_PASS | ||
| - _APP_DB_HOST | ||
| - _APP_DB_PORT | ||
| - _APP_DB_SCHEMA | ||
| - _APP_DB_USER | ||
| - _APP_DB_PASS | ||
| - _APP_SMTP_HOST | ||
| - _APP_SMTP_PORT | ||
| - _APP_SMTP_SECURE | ||
| - _APP_SMTP_USERNAME | ||
| - _APP_SMTP_PASSWORD | ||
| - _APP_USAGE_STATS | ||
| - _APP_STORAGE_LIMIT | ||
| - _APP_STORAGE_PREVIEW_LIMIT | ||
| - _APP_STORAGE_ANTIVIRUS | ||
| - _APP_STORAGE_ANTIVIRUS_HOST | ||
| - _APP_STORAGE_ANTIVIRUS_PORT | ||
| - _APP_STORAGE_DEVICE | ||
| - _APP_STORAGE_S3_ACCESS_KEY | ||
| - _APP_STORAGE_S3_SECRET | ||
| - _APP_STORAGE_S3_REGION | ||
| - _APP_STORAGE_S3_BUCKET | ||
| - _APP_STORAGE_S3_ENDPOINT | ||
| - _APP_STORAGE_DO_SPACES_ACCESS_KEY | ||
| - _APP_STORAGE_DO_SPACES_SECRET | ||
| - _APP_STORAGE_DO_SPACES_REGION | ||
| - _APP_STORAGE_DO_SPACES_BUCKET | ||
| - _APP_STORAGE_BACKBLAZE_ACCESS_KEY | ||
| - _APP_STORAGE_BACKBLAZE_SECRET | ||
| - _APP_STORAGE_BACKBLAZE_REGION | ||
| - _APP_STORAGE_BACKBLAZE_BUCKET | ||
| - _APP_STORAGE_LINODE_ACCESS_KEY | ||
| - _APP_STORAGE_LINODE_SECRET | ||
| - _APP_STORAGE_LINODE_REGION | ||
| - _APP_STORAGE_LINODE_BUCKET | ||
| - _APP_STORAGE_WASABI_ACCESS_KEY | ||
| - _APP_STORAGE_WASABI_SECRET | ||
| - _APP_STORAGE_WASABI_REGION | ||
| - _APP_STORAGE_WASABI_BUCKET | ||
| - _APP_COMPUTE_SIZE_LIMIT | ||
| - _APP_FUNCTIONS_TIMEOUT | ||
| - _APP_SITES_TIMEOUT | ||
| - _APP_COMPUTE_BUILD_TIMEOUT | ||
| - _APP_COMPUTE_CPUS | ||
| - _APP_COMPUTE_MEMORY | ||
| - _APP_FUNCTIONS_RUNTIMES | ||
| - _APP_SITES_RUNTIMES | ||
| - _APP_DOMAIN_SITES | ||
| - _APP_EXECUTOR_SECRET | ||
| - _APP_EXECUTOR_HOST | ||
| - _APP_LOGGING_CONFIG | ||
| - _APP_MAINTENANCE_INTERVAL | ||
| - _APP_MAINTENANCE_DELAY | ||
| - _APP_MAINTENANCE_START_TIME | ||
| - _APP_MAINTENANCE_RETENTION_EXECUTION | ||
| - _APP_MAINTENANCE_RETENTION_CACHE | ||
| - _APP_MAINTENANCE_RETENTION_ABUSE | ||
| - _APP_MAINTENANCE_RETENTION_AUDIT | ||
| - _APP_MAINTENANCE_RETENTION_AUDIT_CONSOLE | ||
| - _APP_MAINTENANCE_RETENTION_USAGE_HOURLY | ||
| - _APP_MAINTENANCE_RETENTION_SCHEDULES | ||
| - _APP_SMS_PROVIDER | ||
| - _APP_SMS_FROM | ||
| - _APP_GRAPHQL_MAX_BATCH_SIZE | ||
| - _APP_GRAPHQL_MAX_COMPLEXITY | ||
| - _APP_GRAPHQL_MAX_DEPTH | ||
| - _APP_VCS_GITHUB_APP_NAME | ||
| - _APP_VCS_GITHUB_PRIVATE_KEY | ||
| - _APP_VCS_GITHUB_APP_ID | ||
| - _APP_VCS_GITHUB_WEBHOOK_SECRET | ||
| - _APP_VCS_GITHUB_CLIENT_SECRET | ||
| - _APP_VCS_GITHUB_CLIENT_ID | ||
| - _APP_MIGRATIONS_FIREBASE_CLIENT_ID | ||
| - _APP_MIGRATIONS_FIREBASE_CLIENT_SECRET | ||
| - _APP_ASSISTANT_OPENAI_API_KEY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Document required environment variables.
The docker-compose configuration references numerous environment variables (e.g., _APP_ENV, _APP_DOMAIN, _APP_DB_HOST, _APP_REDIS_HOST, etc.) that must be provided externally. Consider adding:
- An
.env.examplefile with all required variables and their descriptions - A README or documentation explaining the configuration requirements
- Default values where appropriate
This will help operators deploy the stack correctly without trial-and-error.
Also applies to: 220-236
🤖 Prompt for AI Agents
In appwrite/docker-compose.yml around lines 69-171 (and also apply the same
changes for 220-236): the compose references many external environment variables
without documentation or example values; add an .env.example listing every
_APP_* variable used (grouped by subsystem) with short descriptions and sensible
defaults where safe, update docker-compose.yml to reference the .env file via
env_file and/or provide fallback defaults in the compose file for non-sensitive
values, and add a short README section describing required secret values, how to
populate .env (and secure production secrets), and how to override defaults for
local vs production deployments.
appwrite/docker-compose.yml
Outdated
| - appwrite-builds:/storage/builds:rw | ||
| - appwrite-functions:/storage/functions:rw | ||
| - appwrite-sites:/storage/sites:rw | ||
| # Host mount nessessary to share files between executor and runtimes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in comment.
The word "nessessary" should be "necessary".
Apply this diff:
- # Host mount nessessary to share files between executor and runtimes.
+ # Host mount necessary to share files between executor and runtimes.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Host mount nessessary to share files between executor and runtimes. | |
| # Host mount necessary to share files between executor and runtimes. |
🤖 Prompt for AI Agents
In appwrite/docker-compose.yml around line 877, there's a typo in the comment:
"Host mount nessessary to share files between executor and runtimes." Replace
"nessessary" with "necessary" so the comment reads "Host mount necessary to
share files between executor and runtimes."
appwrite/docker-compose.yml
Outdated
| # It's not possible to share mount file between 2 containers without host mount (copying is too slow) | ||
| - /tmp:/tmp:rw | ||
| environment: | ||
| - OPR_EXECUTOR_INACTIVE_TRESHOLD=$_APP_COMPUTE_INACTIVE_THRESHOLD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify environment variable spelling.
The variable name uses TRESHOLD which is a common misspelling of THRESHOLD. If this is the actual variable name expected by the OpenRuntimes executor, then it's correct. However, if this is a typo, it should be corrected.
🏁 Script executed:
#!/bin/bash
# Check if TRESHOLD or THRESHOLD is used in OpenRuntimes documentation/codebase
# Search for both spellings in the repository
echo "Searching for TRESHOLD usage..."
rg -i "OPR_EXECUTOR_INACTIVE_TRESHOLD" -C2
echo -e "\nSearching for THRESHOLD usage..."
rg -i "OPR_EXECUTOR.*THRESHOLD" -C2
echo -e "\nSearching for any THRESHOLD references..."
rg -i "INACTIVE.*THRESH" -C2Length of output: 1269
**Correct the environment variable name from `TRESHOLD` to `THRESHOLD`.**
The official OpenRuntimes executor documentation specifies `OPR_EXECUTOR_INACTIVE_THRESHOLD` (with correct spelling), but the current line uses `OPR_EXECUTOR_INACTIVE_TRESHOLD`. Update line 881:
- OPR_EXECUTOR_INACTIVE_THRESHOLD=$_APP_COMPUTE_INACTIVE_THRESHOLD
---
🌐 Web query:
```web
OpenRuntimes OPR_EXECUTOR_INACTIVE_TRESHOLD environment variable
💡 Result:
OPR_EXECUTOR_INACTIVE_THRESHOLD is an OpenRuntimes Executor env var that sets the inactivity timeout (in seconds) used to detect and clean up inactive runtimes (example value: 60). [1]
Note: some downstream configs (e.g., Appwrite compose examples) use the misspelled OPR_EXECUTOR_INACTIVE_TRESHOLD (missing the "H"); that appears to be a typo—prefer the documented OPR_EXECUTOR_INACTIVE_THRESHOLD. [2]
Sources:
- OpenRuntimes executor README (environment variables list). [1]
- Example compose/gist showing the misspelled OPR_EXECUTOR_INACTIVE_TRESHOLD. [2]
Correct the misspelled environment variable name to match OpenRuntimes specification.
The correct OpenRuntimes Executor environment variable is OPR_EXECUTOR_INACTIVE_THRESHOLD. Line 881 currently uses OPR_EXECUTOR_INACTIVE_TRESHOLD (missing "H"), which appears to be a typo in downstream configs like Appwrite.
Update line 881 to:
- OPR_EXECUTOR_INACTIVE_THRESHOLD=$_APP_COMPUTE_INACTIVE_THRESHOLD
🤖 Prompt for AI Agents
In appwrite/docker-compose.yml around line 881, the environment variable name is
misspelled as OPR_EXECUTOR_INACTIVE_TRESHOLD; update that single line to use the
correct OpenRuntimes variable name OPR_EXECUTOR_INACTIVE_THRESHOLD and set it to
$_APP_COMPUTE_INACTIVE_THRESHOLD so it matches the spec and downstream configs.
c746635 to
2bceb41
Compare
2bceb41 to
43f33c6
Compare

Moved the Docs link from the footer to the header for easier access while working in the console. This improves navigation flow by keeping frequently used links visible at all times. Updated the BaseNavbar layout for spacing consistency and removed the duplicate footer link. Verified across viewports to ensure proper alignment and responsiveness.
Summary by CodeRabbit