Fix UX mismatch and dead code in paper trading
- Hide exchange dropdown for paper trades (uses single synthetic market) - Add _updateExchangeRowVisibility() to toggle exchange row visibility - Call visibility update on target change and form open - Remove stale 'binance' fallback in Trade constructor call (Trade.__init__ now handles exchange normalization for paper trades) Addresses codex feedback: - Medium: UX mismatch where exchange choice appeared relevant for paper - Low: Dead code passing 'binance' for paper trades Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
2ae087a099
commit
4bfa51f0b1
|
|
@ -19,6 +19,7 @@ class TradeUIManager {
|
||||||
this.stopLossInput = null;
|
this.stopLossInput = null;
|
||||||
this.takeProfitInput = null;
|
this.takeProfitInput = null;
|
||||||
this.timeInForceSelect = null;
|
this.timeInForceSelect = null;
|
||||||
|
this.exchangeRow = null;
|
||||||
this.onCloseTrade = null;
|
this.onCloseTrade = null;
|
||||||
|
|
||||||
// Exchanges known to support testnet/sandbox mode
|
// Exchanges known to support testnet/sandbox mode
|
||||||
|
|
@ -56,7 +57,8 @@ class TradeUIManager {
|
||||||
testnetRowId = 'testnet-row',
|
testnetRowId = 'testnet-row',
|
||||||
stopLossId = 'stopLoss',
|
stopLossId = 'stopLoss',
|
||||||
takeProfitId = 'takeProfit',
|
takeProfitId = 'takeProfit',
|
||||||
timeInForceId = 'timeInForce'
|
timeInForceId = 'timeInForce',
|
||||||
|
exchangeRowId = 'exchange-row'
|
||||||
} = config;
|
} = config;
|
||||||
|
|
||||||
this.targetEl = document.getElementById(targetId);
|
this.targetEl = document.getElementById(targetId);
|
||||||
|
|
@ -83,6 +85,7 @@ class TradeUIManager {
|
||||||
this.stopLossInput = document.getElementById(stopLossId);
|
this.stopLossInput = document.getElementById(stopLossId);
|
||||||
this.takeProfitInput = document.getElementById(takeProfitId);
|
this.takeProfitInput = document.getElementById(takeProfitId);
|
||||||
this.timeInForceSelect = document.getElementById(timeInForceId);
|
this.timeInForceSelect = document.getElementById(timeInForceId);
|
||||||
|
this.exchangeRow = document.getElementById(exchangeRowId);
|
||||||
|
|
||||||
// Set up event listeners
|
// Set up event listeners
|
||||||
this._setupFormListeners();
|
this._setupFormListeners();
|
||||||
|
|
@ -129,10 +132,11 @@ class TradeUIManager {
|
||||||
this.qtyInput.addEventListener('input', updateTradeValue);
|
this.qtyInput.addEventListener('input', updateTradeValue);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Trade target (exchange) changes affect testnet visibility and SELL availability
|
// Trade target (exchange) changes affect testnet visibility, exchange row, and SELL availability
|
||||||
if (this.targetSelect) {
|
if (this.targetSelect) {
|
||||||
this.targetSelect.addEventListener('change', () => {
|
this.targetSelect.addEventListener('change', () => {
|
||||||
this._updateTestnetVisibility();
|
this._updateTestnetVisibility();
|
||||||
|
this._updateExchangeRowVisibility();
|
||||||
this._updateSellAvailability();
|
this._updateSellAvailability();
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
@ -298,6 +302,25 @@ class TradeUIManager {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Updates exchange row visibility based on trade mode.
|
||||||
|
* Paper trades use a single synthetic market, so exchange selection is irrelevant.
|
||||||
|
*/
|
||||||
|
_updateExchangeRowVisibility() {
|
||||||
|
if (!this.exchangeRow || !this.targetSelect) return;
|
||||||
|
|
||||||
|
const selectedTarget = this.targetSelect.value;
|
||||||
|
const isPaperTrade = selectedTarget === 'test_exchange';
|
||||||
|
|
||||||
|
if (isPaperTrade) {
|
||||||
|
// Hide exchange row for paper trading (uses single synthetic market)
|
||||||
|
this.exchangeRow.style.display = 'none';
|
||||||
|
} else {
|
||||||
|
// Show exchange row for live exchanges
|
||||||
|
this.exchangeRow.style.display = 'contents';
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Populates the exchange selector with connected exchanges.
|
* Populates the exchange selector with connected exchanges.
|
||||||
* @param {string[]} connectedExchanges - List of connected exchange names.
|
* @param {string[]} connectedExchanges - List of connected exchange names.
|
||||||
|
|
@ -404,13 +427,17 @@ class TradeUIManager {
|
||||||
const exchangeToUse = chartExchange || 'binance';
|
const exchangeToUse = chartExchange || 'binance';
|
||||||
await this._populateSymbolDropdown(exchangeToUse, symbol);
|
await this._populateSymbolDropdown(exchangeToUse, symbol);
|
||||||
|
|
||||||
// Reset to paper trade and hide testnet row
|
// Reset to paper trade and hide testnet/exchange rows
|
||||||
if (this.targetSelect) {
|
if (this.targetSelect) {
|
||||||
this.targetSelect.value = 'test_exchange';
|
this.targetSelect.value = 'test_exchange';
|
||||||
}
|
}
|
||||||
if (this.testnetRow) {
|
if (this.testnetRow) {
|
||||||
this.testnetRow.style.display = 'none';
|
this.testnetRow.style.display = 'none';
|
||||||
}
|
}
|
||||||
|
if (this.exchangeRow) {
|
||||||
|
// Hide exchange row for paper trading (uses single synthetic market)
|
||||||
|
this.exchangeRow.style.display = 'none';
|
||||||
|
}
|
||||||
if (this.testnetCheckbox) {
|
if (this.testnetCheckbox) {
|
||||||
this.testnetCheckbox.checked = true;
|
this.testnetCheckbox.checked = true;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -31,11 +31,13 @@
|
||||||
</small>
|
</small>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<!-- Exchange Selection -->
|
<!-- Exchange Selection (for price data source - hidden for paper trades) -->
|
||||||
<label for="tradeExchange"><b>Exchange:</b></label>
|
<div id="exchange-row" style="display: contents;">
|
||||||
<select name="tradeExchange" id="tradeExchange">
|
<label for="tradeExchange"><b>Exchange:</b></label>
|
||||||
<!-- Exchanges populated dynamically, defaults to chart view -->
|
<select name="tradeExchange" id="tradeExchange">
|
||||||
</select>
|
<!-- Exchanges populated dynamically, defaults to chart view -->
|
||||||
|
</select>
|
||||||
|
</div>
|
||||||
|
|
||||||
<!-- Symbol Selection -->
|
<!-- Symbol Selection -->
|
||||||
<label for="tradeSymbol"><b>Symbol:</b></label>
|
<label for="tradeSymbol"><b>Symbol:</b></label>
|
||||||
|
|
|
||||||
|
|
@ -685,9 +685,10 @@ class Trades:
|
||||||
trade_status = status_map.get(result.status, 'pending')
|
trade_status = status_map.get(result.status, 'pending')
|
||||||
|
|
||||||
# Create Trade with full broker tracking
|
# Create Trade with full broker tracking
|
||||||
|
# Note: Trade.__init__ normalizes exchange to 'paper' for paper trades
|
||||||
trade = Trade(
|
trade = Trade(
|
||||||
target=target,
|
target=target,
|
||||||
exchange=exchange or (target if not is_paper else 'binance'),
|
exchange=exchange,
|
||||||
symbol=symbol,
|
symbol=symbol,
|
||||||
side=side.upper(),
|
side=side.upper(),
|
||||||
order_price=effective_price,
|
order_price=effective_price,
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue