Add search parameter to get all tokens function in the sdk#2045
Merged
Add search parameter to get all tokens function in the sdk#2045
Conversation
Contributor
WalkthroughAdded an optional search parameter to the SDK's get_all_tokens and updated UI components to call gui.getAllTokens(search) asynchronously; removed prop-driven token lists and client-side filtering, refactored TokenSelectionModal to fetch/filter tokens, and updated tests to use the new API. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Svelte Component (Modal / Select)
participant GUI as DotrainOrderGui (JS API)
participant SDK as Rust SDK
UI->>GUI: getAllTokens(searchTerm?)
GUI->>SDK: get_all_tokens(searchTerm?)
SDK-->>GUI: filtered TokenInfo[]
GUI-->>UI: TokenInfo[] (render)
UI->>UI: render list / show no-results / focus input
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ 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). (8)
🔇 Additional comments (4)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Comment on lines
+18
to
+29
| async function loadTokens(search?: string) { | ||
| isSearching = true; | ||
|
|
||
| const result = await gui.getAllTokens(search); | ||
| if (result.error) { | ||
| tokens = []; | ||
| return; | ||
| } | ||
| tokens = result.value; | ||
|
|
||
| isSearching = false; | ||
| } |
Contributor
There was a problem hiding this comment.
Fix loading state management in error cases.
The isSearching flag is only set to false in the success path. If an error occurs, the loading state will persist indefinitely, potentially confusing users.
Apply this fix to ensure the loading state is always cleared:
async function loadTokens(search?: string) {
isSearching = true;
-
- const result = await gui.getAllTokens(search);
- if (result.error) {
- tokens = [];
- return;
- }
- tokens = result.value;
-
- isSearching = false;
+ try {
+ const result = await gui.getAllTokens(search);
+ if (result.error) {
+ tokens = [];
+ return;
+ }
+ tokens = result.value;
+ } finally {
+ isSearching = false;
+ }
}📝 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.
Suggested change
| async function loadTokens(search?: string) { | |
| isSearching = true; | |
| const result = await gui.getAllTokens(search); | |
| if (result.error) { | |
| tokens = []; | |
| return; | |
| } | |
| tokens = result.value; | |
| isSearching = false; | |
| } | |
| async function loadTokens(search?: string) { | |
| isSearching = true; | |
| try { | |
| const result = await gui.getAllTokens(search); | |
| if (result.error) { | |
| tokens = []; | |
| return; | |
| } | |
| tokens = result.value; | |
| } finally { | |
| isSearching = false; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
packages/ui-components/src/lib/components/deployment/TokenSelectionModal.svelte
around lines 18 to 29, the isSearching flag is set to false only when the token
loading succeeds, but not when an error occurs, causing the loading state to
persist indefinitely. To fix this, ensure isSearching is set to false in both
success and error paths by moving the assignment to false after the error check
or using a finally block to guarantee it always resets after the async operation
completes.
Comment on lines
+52
to
+59
| $: if (modalOpen) { | ||
| tick().then(() => { | ||
| const input = document.querySelector('.token-search-input') as HTMLInputElement; | ||
| if (input) { | ||
| input.focus(); | ||
| } | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Auto-focus implementation addresses user experience requirements.
The auto-focus functionality correctly implements the requirement from issue #2034, allowing users to start typing immediately when the modal opens. The use of tick() ensures DOM updates are complete before focusing.
Optional improvement: Consider using a more specific selector or Svelte's bind:this to make the element reference more robust:
+let searchInput: HTMLInputElement;
+
$: if (modalOpen) {
tick().then(() => {
- const input = document.querySelector('.token-search-input') as HTMLInputElement;
- if (input) {
- input.focus();
- }
+ searchInput?.focus();
});
}
// Then in the template:
-<Input ... class="token-search-input pl-10" />
+<Input ... bind:this={searchInput} class="pl-10" />📝 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.
Suggested change
| $: if (modalOpen) { | |
| tick().then(() => { | |
| const input = document.querySelector('.token-search-input') as HTMLInputElement; | |
| if (input) { | |
| input.focus(); | |
| } | |
| }); | |
| } | |
| <script lang="ts"> | |
| import { tick } from 'svelte'; | |
| export let modalOpen: boolean; | |
| + let searchInput: HTMLInputElement; | |
| $: if (modalOpen) { | |
| tick().then(() => { | |
| - const input = document.querySelector('.token-search-input') as HTMLInputElement; | |
| - if (input) { | |
| - input.focus(); | |
| - } | |
| + searchInput?.focus(); | |
| }); | |
| } | |
| </script> | |
| <!-- Later in the markup --> | |
| -<Input … class="token-search-input pl-10" /> | |
| +<Input … bind:this={searchInput} class="pl-10" /> |
🤖 Prompt for AI Agents
In
packages/ui-components/src/lib/components/deployment/TokenSelectionModal.svelte
around lines 52 to 59, the current auto-focus uses document.querySelector with a
class selector which can be fragile. To improve robustness, replace the
querySelector with Svelte's bind:this directive to directly reference the input
element. Declare a variable to hold the input element reference, bind it to the
input in the markup, and then call focus() on that variable inside the tick()
callback when modalOpen is true.
Collaborator
Author
There was a problem hiding this comment.
this does not work. that why i had to use query selector.
Contributor
There was a problem hiding this comment.
@findolor Thank you for the clarification! You're absolutely right - sometimes the "cleaner" approach doesn't work in practice, especially when dealing with third-party components like Flowbite's Input.
The querySelector approach is a pragmatic solution that works reliably in this case. Component libraries can have their own internal structure and lifecycle that doesn't always play nicely with Svelte's bind:this, particularly in modal contexts where DOM timing can be tricky.
Your implementation correctly handles the auto-focus requirement from issue #2034, and if querySelector is what works here, then that's the right approach.
✏️ Learnings added
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2045
File: packages/ui-components/src/lib/components/deployment/TokenSelectionModal.svelte:52-59
Timestamp: 2025-07-29T12:30:44.304Z
Learning: In packages/ui-components/src/lib/components/deployment/TokenSelectionModal.svelte, the bind:this approach doesn't work for auto-focusing the Flowbite Input component when the modal opens, requiring the use of document.querySelector('.token-search-input') instead as a working solution.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1925
File: packages/ui-components/src/lib/components/deployment/SelectToken.svelte:137-151
Timestamp: 2025-06-18T16:44:14.948Z
Learning: In the SelectToken.svelte component, the SDK validates addresses before making RPC calls, so calling saveTokenSelection on every keystroke in handleInput doesn't result in network calls until there's a full valid address.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1493
File: packages/ui-components/src/lib/components/deployment/DeployButton.svelte:0-0
Timestamp: 2025-03-31T14:01:19.067Z
Learning: The DeployButton component in packages/ui-components/src/lib/components/deployment/DeployButton.svelte is designed to be generic, dispatching clickDeploy events to be handled at the page level, where validation of event details (result, networkKey, subgraphUrl) occurs.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1974
File: packages/ui-components/src/__tests__/DeploymentSteps.test.ts:123-126
Timestamp: 2025-07-09T12:35:45.699Z
Learning: In packages/ui-components/src/__tests__/DeploymentSteps.test.ts, findolor prefers to keep mock initializations (like setSelectToken) in individual test cases rather than consolidating them into shared beforeEach blocks, even when it results in duplication.
Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2002
File: tauri-app/src/lib/components/ModalMultipleVaultsWithdraw.svelte:20-25
Timestamp: 2025-07-24T08:16:47.393Z
Learning: In the Rain OrderBook Tauri app, modal components directly mutate the `open` prop (setting `open = false`) as a consistent pattern across the codebase. This approach is maintained for consistency even though it violates typical Svelte one-way data flow, with plans to refactor all modals together in the future rather than making isolated changes.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1870
File: packages/webapp/src/__tests__/WithdrawModal.test.ts:291-303
Timestamp: 2025-05-20T12:05:48.570Z
Learning: The modal closing behavior in the WithdrawModal component is handled by Flowbite's native implementation rather than through component props, making it unnecessary to test the open state changes directly.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2004
File: packages/ui-components/src/lib/components/input/InputRegistryUrl.svelte:29-33
Timestamp: 2025-07-17T13:33:29.619Z
Learning: In packages/ui-components/src/lib/components/input/InputRegistryUrl.svelte, findolor is comfortable keeping the input element id as "strategy-url" even when the placeholder text has been updated to reference "order", indicating acceptance of mixed terminology during the strategy-to-order rename transition.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1493
File: packages/ui-components/src/__tests__/DeployButton.test.ts:36-40
Timestamp: 2025-03-28T10:22:11.771Z
Learning: The DeployButton component in packages/ui-components/src/lib/components/deployment/DeployButton.svelte doesn't have an explicit disabled prop. Instead, its disabled state is internally controlled by the checkingDeployment variable, which is set to true during deployment checks.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1925
File: packages/ui-components/src/lib/errors/DeploymentStepsError.ts:16-16
Timestamp: 2025-07-10T12:52:47.468Z
Learning: In packages/ui-components/src/lib/errors/DeploymentStepsError.ts, the error codes NO_SELECT_TOKENS ('Error loading tokens') and NO_AVAILABLE_TOKENS ('Error loading available tokens') represent different failure scenarios in the token loading workflow and should remain as separate error codes.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1925
File: packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte:87-104
Timestamp: 2025-06-18T16:42:40.608Z
Learning: In the orderbook UI components, changing deployments or networks requires page navigation, which automatically remounts components and refreshes data like available tokens. No additional refresh logic is needed for deployment/network changes.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1651
File: packages/ui-components/src/lib/components/TanstackAppTable.svelte:13-13
Timestamp: 2025-04-10T16:32:51.198Z
Learning: In the TanstackAppTable.svelte component, the queryKey prop should be required (type: string) rather than optional (string | undefined) to ensure proper query invalidation.
Learnt from: brusherru
PR: rainlanguage/rain.orderbook#1957
File: packages/ui-components/src/lib/components/tables/OrdersListTable.svelte:86-94
Timestamp: 2025-06-27T18:41:18.194Z
Learning: In the rain.orderbook UI components, the tokensQuery should use `enabled: true` rather than `Object.keys($activeSubgraphs).length > 0` because tokens are needed for filtering functionality even when no specific subgraphs are selected. The multiSubgraphArgs falls back to default subgraphs from settings, so disabling the query when no subgraphs are active would break the token filtering feature.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1512
File: packages/ui-components/src/__tests__/OrderDetail.test.ts:0-0
Timestamp: 2025-04-09T09:28:05.097Z
Learning: The OrderDetail component has been refactored to use an `onRemove` callback approach instead of the previous `handleOrderRemoveModal` pattern for order removal functionality, as part of PR #1512.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1925
File: crates/settings/src/remote_tokens.rs:44-62
Timestamp: 2025-07-11T08:45:43.961Z
Learning: findolor prefers to avoid performance micro-optimizations like changing `.values()` to `.into_values()` in HashMap iterations in the rainlanguage/rain.orderbook codebase, even when they would eliminate unnecessary cloning.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1903
File: crates/js_api/src/subgraph/vault.rs:129-138
Timestamp: 2025-06-17T16:33:58.549Z
Learning: findolor prefers downstream error handling over early validation checks for empty RPC lists in vault functions, allowing errors to bubble up from provider creation logic naturally.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1916
File: packages/webapp/src/routes/+layout.ts:26-26
Timestamp: 2025-06-10T12:01:07.418Z
Learning: The user findolor prefers to keep string concatenation using the plus operator over template literals in the rainlanguage/rain.orderbook project, even when static analysis tools suggest the change.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1975
File: crates/js_api/src/config.rs:44-45
Timestamp: 2025-07-09T07:42:48.458Z
Learning: User findolor prefers to defer code improvements and refactoring suggestions to future PRs when they are not directly related to the current PR's scope, even when the suggestions are valid improvements.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1947
File: crates/common/src/raindex_client/orders.rs:462-1643
Timestamp: 2025-06-24T08:45:10.971Z
Learning: User findolor prefers to keep tests simple and avoid refactoring for maintainability when the code is stable and viewed as "one time thing" that won't require frequent changes.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2001
File: crates/common/src/raindex_client/order_quotes.rs:62-69
Timestamp: 2025-07-16T14:33:13.457Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor considers hardcoded decimal values (18 and 36) in order quote formatting logic to be acceptable for their use case, even when dynamic token decimals could theoretically provide more accurate formatting for different tokens.
Collaborator
There was a problem hiding this comment.
this gives me jQuery PTSD flashbacks but if that's what works then fair enough
Contributor
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
0xgleb
approved these changes
Aug 6, 2025
Comment on lines
+52
to
+59
| $: if (modalOpen) { | ||
| tick().then(() => { | ||
| const input = document.querySelector('.token-search-input') as HTMLInputElement; | ||
| if (input) { | ||
| input.focus(); | ||
| } | ||
| }); | ||
| } |
Collaborator
There was a problem hiding this comment.
this gives me jQuery PTSD flashbacks but if that's what works then fair enough
hardyjosh
approved these changes
Aug 13, 2025
Contributor
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. SIZE=M 🧠 Learnings used |
This was referenced Dec 22, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
See issues: #1948 and #2034
The getAllTokens function was limited to returning all available tokens without any filtering capability. This made it difficult for users to quickly find specific tokens in large token lists, especially when searching by name, symbol, or address.
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
fix #1948
fix #2034
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests