-
Notifications
You must be signed in to change notification settings - Fork 197
Fix: input selects #2597
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
Fix: input selects #2597
Conversation
Console (appwrite/console)Project ID: Tip Git integration provides automatic deployments with optional PR comments |
WalkthroughThe pull request updates two dependency references in package.json: Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
27-27: Consider using semantic versioning or published releases instead of commit hashes.Using git commit hashes as dependency versions is unconventional and reduces maintainability. Commit hashes are opaque to developers, harder to audit in changelogs, and increase friction during upgrades.
If bd82d9a represents a stable fix:
- Check if the libraries have a published release/tag you can pin to instead
- Otherwise, consider documenting why a commit hash is necessary (e.g., fix not yet released)
- Ensure this is not a temporary pin that should be upgraded once a version is released
Based on learnings: The retrieved learning from PR #2567 mentions fixes related to Input.Select event handling in pink-svelte. Confirm that this update (bd82d9a) includes those fixes and any downstream code relying on
HTMLInputElementtype casting is compatible.Also applies to: 29-29
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2567
File: src/lib/elements/forms/inputSelect.svelte:26-36
Timestamp: 2025-11-06T06:47:24.645Z
Learning: In the appwrite.io/pink-svelte library, the Input.Select component internally uses an `input` element (not a `select` element) that triggers the `on:input` event, so event.target should be cast as HTMLInputElement in event handlers.
📚 Learning: 2025-09-26T06:48:57.938Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:629-631
Timestamp: 2025-09-26T06:48:57.938Z
Learning: In the Appwrite console codebase using appwrite.io/pink-svelte, the Icon component automatically handles CSS variable names passed to its color prop by internally wrapping them with var(). Therefore, passing '--some-css-variable' as a string to the Icon color prop works correctly without needing to manually wrap it with var().
Applied to files:
package.json
🔇 Additional comments (1)
package.json (1)
27-27: Provide concrete PR description and clarify why commit hashes are used instead of semantic versions.The packages @appwrite.io/pink-svelte (2.0.0-RC.2) and @appwrite.io/pink-icons-svelte (2.0.0-RC.1) publish to npm with semantic versioning, making the use of git commit hashes (4472521 → bd82d9a) via pkg.vc unusual and opaque.
The PR description is missing—it contains only placeholder template text. Please:
- Update the PR description to explain:
- What specific problem exists with input selects
- How commit bd82d9a addresses it
- Any breaking changes or side effects
- Why commit hashes are used instead of semantic versions or RC releases from npm
- Link to any related issues or previous PRs documenting the problem
- Clarify whether bd82d9a is temporary pre-release work or will become a published semantic version release
Also applies to: 29-29

What does this PR do?
Fixes #2596
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit