Allow scrolling outstanding orders #47
Conversation
No semantic changes made
Since we're adding interactivity (scrolling) to the orders, we want to exit the process based on user action instead of after command is finished running
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR primarily adds scrolling functionality to the orders list view and implements consistent code formatting across the codebase.
- Added
ScrollAreacomponent insrc/lib/orders/OrderDisplay.tsxfor keyboard-navigable order list viewing - Implemented tabbed filtering (all/buy/sell) for orders display with improved date/time formatting
- Added sorting by start time in ascending order for orders list in
src/lib/orders/index.tsx - Added
.prettierrcconfiguration file to standardize code formatting rules project-wide - Added local development setup instructions in README.md for contributor onboarding
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
31 file(s) reviewed, 25 comment(s)
Edit PR Review Bot Settings | Greptile
| const response = await fetch( | ||
| "https://raw.githubusercontent.com/sfcompute/cli/refs/heads/main/package.json", | ||
| "https://raw.githubusercontent.com/sfcompute/cli/refs/heads/main/package.json" | ||
| ); |
There was a problem hiding this comment.
style: consider using a constant for the GitHub URL to make it more maintainable and reusable across the codebase
| - Install dependencies `deno install` | ||
| - Use same mental model as `npm install` |
There was a problem hiding this comment.
logic: the deno install command is incorrect - Deno uses deno cache to install dependencies. Also, the npm mental model comparison may be confusing since Deno handles dependencies differently
| - Install [Deno](https://docs.deno.com/runtime/) | ||
| - Install dependencies `deno install` | ||
| - Use same mental model as `npm install` | ||
| - Auth your CLI with `deno run prod login` |
There was a problem hiding this comment.
logic: missing prerequisite step to set up local environment variables or configuration before running login
| - `deno run devv` to test against local API | ||
| - `deno run prod` to test against production API |
There was a problem hiding this comment.
syntax: typo in command - devv should be dev based on the codebase
| - Test changes with | ||
| - `deno run devv` to test against local API | ||
| - `deno run prod` to test against production API | ||
| - The `deno run <env>` is an alias to the user facing `sf` command. So if you wanted to run `sf login` locally against the local API, run `deno run devv login` |
There was a problem hiding this comment.
syntax: example is inconsistent - uses devv when it should be dev to match the command above
| const date = epochToDate(Number.parseInt(epochTimestamp)); | ||
| console.log( | ||
| `${colorDiffedEpochs[i]} | ${ | ||
| chalk.yellow( | ||
| dayjs(date).format("hh:mm A MM-DD-YYYY"), | ||
| ) | ||
| } Local`, | ||
| `${colorDiffedEpochs[i]} | ${chalk.yellow( | ||
| dayjs(date).format("hh:mm A MM-DD-YYYY") | ||
| )} Local` | ||
| ); |
There was a problem hiding this comment.
logic: parsing epoch timestamps without validation could cause runtime errors if invalid strings are provided
| @@ -32,9 +32,9 @@ export function registerSell(program: Command) { | |||
| .option( | |||
| "-f, --flags <flags>", | |||
| "Specify additional flags as JSON", | |||
| JSON.parse, | |||
| JSON.parse | |||
There was a problem hiding this comment.
logic: JSON.parse used directly without try/catch could crash on invalid JSON input
| @@ -84,15 +84,14 @@ async function placeSellOrder(options: { | |||
|
|
|||
| if (contract?.status === "pending") { | |||
There was a problem hiding this comment.
style: optional chaining on contract?.status is redundant since contract existence was already checked
| const spinner = ora(); | ||
|
|
||
| if (version) { | ||
| spinner.start(`Checking if version ${version} exists`); | ||
| const url = | ||
| `https://github.com/sfcompute/cli/archive/refs/tags/${version}.zip`; | ||
| const url = `https://github.com/sfcompute/cli/archive/refs/tags/${version}.zip`; | ||
| const response = await fetch(url, { method: "HEAD" }); |
There was a problem hiding this comment.
logic: no error handling for network failures when checking version - wrap in try/catch
| const scriptResponse = await fetch( | ||
| "https://www.sfcompute.com/cli/install", | ||
| "https://www.sfcompute.com/cli/install" | ||
| ); |
There was a problem hiding this comment.
style: no timeout set for fetch request - could hang indefinitely if server is unresponsive
Context: https://sfcompute.slack.com/archives/C079B9UE2LE/p1735947098013759
This PR also adds a prettier config. A bunch of files were changed because formatting was applied. Should review these 2 commits that introduce functional changes