Small fixes - #16
Conversation
There was a problem hiding this comment.
Code Review
This pull request removes the scope navigation items from the top navigation bar, updates chart crosshair and background styling, and adjusts the default builder fee configuration. It also introduces a new IndexPage component that sets a default market on mount. Feedback highlights a mathematical discrepancy in the fee rate update where the value and comment changes are inconsistent, and notes that the unconditional market reset in the index route may negatively impact user experience by overriding persisted selections.
| export const DEFAULT_BUILDER_CONFIG: BuilderConfig = { | ||
| b: "0xe2E7BFE54DDfe5983807C187A57A939C33Eaeb2e", // builder | ||
| f: 1, // fee rate 0.001% | ||
| f: 1000, // fee rate 0.1% |
There was a problem hiding this comment.
There is a mathematical inconsistency between the value change and the comment change. The value increased 1000x (from 1 to 1000), while the comment indicates a 100x increase in the fee rate (from 0.001% to 0.1%). If the intended fee is 0.1% and the previous scale was correct, the value should be 100. Please ensure the value aligns with the intended fee rate and the protocol's expected units.
| useEffect(() => { | ||
| setSelectedMarket("all", "VOLX-USDH"); | ||
| }, [setSelectedMarket]); |
There was a problem hiding this comment.
This useEffect unconditionally resets the selected market to VOLX-USDH whenever the index page is mounted. This overrides the user's persisted market selection from useMarketStore, which can lead to a confusing user experience when navigating back to the home route. Consider implementing logic to only set a default if no market is currently selected or if this is the intended behavior for the landing page.
No description provided.