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

Futures mobile fixes #1175

Merged
merged 24 commits into from
Aug 2, 2022
Merged

Futures mobile fixes #1175

merged 24 commits into from
Aug 2, 2022

Conversation

koredefashokun
Copy link
Contributor

Description

This PR fixes a number of reported issues with futures on mobile.

Related issue

N/A

Motivation and Context

Conclusion of mobile compatibility development (until new features are added).

How Has This Been Tested?

N/A

Screenshots (if appropriate):

@vercel
Copy link

vercel bot commented Jul 25, 2022

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

Name Status Preview Updated
kwenta ✅ Ready (Inspect) Visit Preview Sep 2, 2022 at 9:19PM (UTC)

@platschi platschi added this to the Mobile milestone Jul 25, 2022
@platschi platschi added core dev tickest suitable for core develpers enhancement New features or improvements to existing code labels Jul 25, 2022
@vercel vercel bot temporarily deployed to Preview July 26, 2022 01:34 Inactive
@vercel vercel bot temporarily deployed to Preview July 26, 2022 15:37 Inactive
@koredefashokun koredefashokun marked this pull request as ready for review July 26, 2022 16:21
@vercel vercel bot temporarily deployed to Preview July 26, 2022 16:23 Inactive
@vercel vercel bot temporarily deployed to Preview July 27, 2022 03:33 Inactive
@platschi
Copy link
Collaborator

platschi commented Jul 27, 2022

Looks better now! A few comments:

  • Top buttons on Markets (Price and Trades) shift layout when switching from either Account or Stats (see screen recording attached). Note Add Skew data in mobile market stats tab #1184 adds more data to the Stats, so maybe merge that one first to avoid fixing the layout shift twice.

  • Price button should be moved into the first place and load on default

  • When scrolling down on Dashboard and selecting a market (e.g. try with the last one in the list), page jumps to the middle of the market page, not to the top (as would be expected)

  • On Dashboard, Futures Markets Stats, rename "Total Trades" to "24h Trades"

  • On Markets, Position Card should be hidden if users don't have a position open (this is the case when wallet not connected, but with a connected wallet the empty Position Card is still visible)

  • Still being discussed: Make Dashboard menu link clickable (link straight to Overview)

  • Need to set up individual Markets page (similar to Desktop), as currently "Overview" and "Markets" links in navigation point to the same page (Dashboard). This will be tracked in a separate ticket. (#1187)

RPReplay_Final1658931098.MP4

@vercel vercel bot temporarily deployed to Preview July 27, 2022 20:04 Inactive
@vercel vercel bot temporarily deployed to Preview July 28, 2022 09:51 Inactive
@vercel vercel bot temporarily deployed to Preview July 28, 2022 10:05 Inactive
@vercel vercel bot temporarily deployed to Preview July 29, 2022 08:05 Inactive
@vercel vercel bot temporarily deployed to Preview July 29, 2022 08:22 Inactive
@platschi
Copy link
Collaborator

platschi commented Jul 29, 2022

  • Left-align "Oracle/Entry" column data in Open Position table to avoid shifts with higher amounts (see Discord channel comments):

How it is

shift1

How it should be

shift

@vercel vercel bot temporarily deployed to Preview July 30, 2022 08:13 Inactive
@vercel vercel bot temporarily deployed to Preview July 30, 2022 09:42 Inactive
@vercel vercel bot temporarily deployed to Preview July 31, 2022 21:04 Inactive
@vercel vercel bot temporarily deployed to Preview July 31, 2022 21:30 Inactive
@platschi
Copy link
Collaborator

platschi commented Aug 1, 2022

Looks great now, much better with the buttons not jumping around! Re: markets not jumping to top, we'll create a separate ticket for this.

My two last comments:

  1. Skew values >1M create additional rows which makes the Stats table border overlap with the buttons. see screenshot:

skew

  1. Open Position price box is now nicely aligned, however leverage values >=10.00x create an obsolete new line. Maybe expand the left column width a bit so it fits nicely. See screnshot:

Screen Shot 2022-08-01 at 15 08 53

@vercel vercel bot temporarily deployed to Preview August 2, 2022 03:03 Inactive
@vercel vercel bot temporarily deployed to Preview August 2, 2022 07:58 Inactive
@platschi platschi requested a review from Tburm August 2, 2022 12:54
platschi
platschi previously approved these changes Aug 2, 2022
Copy link
Collaborator

@platschi platschi left a comment

Choose a reason for hiding this comment

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

Thanks, all changes work good now!

@platschi
Copy link
Collaborator

platschi commented Aug 2, 2022

From @Marko-roy:

  1. "Orders" Size amount on mobile: Remove asset name (should be the same as on desktop) to just show the Size value.
  2. Column "Actions" not aligned with "Cancel"
  3. "Orders" tab table Type not aligned on mobile, same with "Short" box, text should be centered and box padding aligned. See screenshot:

IMG_4097

@vercel vercel bot temporarily deployed to Preview August 2, 2022 20:56 Inactive
@vercel vercel bot temporarily deployed to Preview August 2, 2022 21:19 Inactive
@platschi platschi merged commit 61c0a6d into dev Aug 2, 2022
@platschi platschi deleted the futures-mobile-fixes branch August 2, 2022 22:45
@KngZhi KngZhi mentioned this pull request Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core dev tickest suitable for core develpers enhancement New features or improvements to existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants