Skip to content
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

feat: only show twap market price for non final states #5302

Merged
merged 7 commits into from
Jan 16, 2025

Conversation

fairlighteth
Copy link
Contributor

@fairlighteth fairlighteth commented Jan 16, 2025

Show market price only for active orders

  • Refactors market price rendering logic to only display prices for non-finalized orders.
  • Replaces nested ternaries with clear conditional blocks for better readability and maintenance.
  • Parent and child orders now consistently show '-' when in final states.

Copy link

vercel bot commented Jan 16, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
cowfi ✅ Ready (Inspect) Visit Preview Jan 16, 2025 5:09pm
explorer-dev 🛑 Canceled (Inspect) Jan 16, 2025 5:09pm
swap-dev ✅ Ready (Inspect) Visit Preview Jan 16, 2025 5:09pm
3 Skipped Deployments
Name Status Preview Updated (UTC)
cosmos ⬜️ Ignored (Inspect) Visit Preview Jan 16, 2025 5:09pm
sdk-tools ⬜️ Ignored (Inspect) Visit Preview Jan 16, 2025 5:09pm
widget-configurator ⬜️ Ignored (Inspect) Visit Preview Jan 16, 2025 5:09pm

'-'
) : spotPrice ? (
const renderMarketPrice = () => {
// Early return for cancelled, warning, or presignature pending states
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, and not from this PR, but withWarning should be the latest or first condition, as it reads more natural, state in CANCELED, PRESIGN or has a warning

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed this now


// Check children finalization status
if (children) {
const childrenArray = React.Children.toArray(children) as React.ReactElement<{ order: ParsedOrder }>[]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with react documentation

image

Can't we reason using just the data props instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed this

Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @fairlighteth , great! However, still for some Expired orders I can see a market price.
Examples:

Sepolia:
image

Could you please check why?

@fairlighteth
Copy link
Contributor Author

@elena-zh the issues should be fixed now:
Screenshot 2025-01-16 at 15 56 22
Screenshot 2025-01-16 at 15 56 13

@alfetopito alfetopito force-pushed the feat/limit-ui-upgrade-12 branch from 1a6e5f0 to 3ad7920 Compare January 16, 2025 16:04
Base automatically changed from feat/limit-ui-upgrade-12 to feat/limit-ui-upgrade-2 January 16, 2025 16:06
Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now!

* feat: hide orders table widget

* feat: hide orders table widget for limit and twap
@alfetopito alfetopito merged commit 9b7c816 into feat/limit-ui-upgrade-2 Jan 16, 2025
12 of 13 checks passed
@alfetopito alfetopito deleted the feat/limit-ui-upgrade-13 branch January 16, 2025 18:11
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants