[Explorer] Orders table responsive #721

matextrem · 2021-10-04T22:07:43Z

Summary

Closes #693

Updated styles and some markup in order to support new design for Orders table to be responsive.

To Test

Open the page /rinkeby/address/<any address with orders>/ (i.e 0xb6bad41ae76a11d10f7b0e664c5007b908bc77c9)

View it in mobile view
Design will change in order tu support responsive behavior

github-actions · 2021-10-04T22:12:54Z

🔭 Explorer: Explorer test app
🔒 Safe Swap App: Trade test app (add to your safe apps)
📈 Trade: Trade test app
📚 Storybook: Component stories

alfetopito

Few comments regarding the code.

Regarding the behaviour:

When inverting the price, only the selected element is inverted

I understand why it would make sense to have it like that, but I rather keep the same behaviour on both modes

Some wrapping issues with the copy button

Maybe make the parent component non-wrappable?

Other than that, quite good!

@biocom would you mind reviewing the CSS as well?

src/components/orders/OrdersUserDetailsTable/index.tsx

fairlighteth · 2021-10-05T07:38:27Z

Some suggestions on the responsive card:

I'd increase the gap between the each horizontal line
In crease the gap between label > value, and account for forceful text wrapping.
The 'copied' feedback message should use 100% width on top.

src/apps/explorer/components/common/ExplorerTabs/ExplorerTab.tsx

fairlighteth · 2021-10-05T07:54:12Z

src/components/orders/OrdersUserDetailsTable/index.tsx

+    display: none;
+  }
+
+  @media ${MEDIA.mobile} {


Move the media query inside the existing selector on lines 22/23

Better:

Turn the table into a styled component or import an existing table. This keeps both the Wrapper component and table selector dryer.

Better:

Turn the table into a styled component or import an existing table. This keeps both the Wrapper component and table selector dryer.

I don't understand this. Wrapper component is already a Table styled component.

Move the media query inside the existing selector on lines 22/23

But the selector used in 22/23 is for common styles for thead and tr. Moving the media query there would not work the same.

src/components/orders/OrdersUserDetailsTable/index.tsx

elena-zh · 2021-10-05T11:18:59Z

Some notes from my side:

There is no pagination in the table: a user is able to see only 20 first orders
Not a priority, but it would be nice to show a tooltip arrow right under/below the 'question' icon
It would be nice to apply 'mobile' style to a tablet devices, I think. I mean, when a screen width is in [721-1050] px, the table becomes scrollable, transaction status is hidden. Seems to be a not convenient a bit, especially when we add 'Cancel transaction' feature to Explorer in the future.
I'd remove 'violet' color when I click on an order in a mobile view. The card is highlighted with violet when I press on a tooltip, or when I scroll the list and accidentally tough it.. Looks a bit strange to me, frankly.

@alfetopito , @alongoni , @biocom , WDYT?

Thanks

fairlighteth · 2021-10-05T11:34:29Z

Could not show the hover style for mobile/touch/responsive.

alfetopito · 2021-10-05T15:21:35Z

@alongoni @biocom one extra question that came up is regarding the "invert price" button behaviour on mobile, and I'd like a UX input from you guys.

What do you think would be the best approach on mobile when clicked?

invert only the current order price
invert all table prices - like the desktop behaviour

alongoni · 2021-10-05T17:32:16Z

Some notes from my side:

There is no pagination in the table: a user is able to see only 20 first orders

The pagination is there but hidden 😅, maybe we can show it.

Not a priority, but it would be nice to show a tooltip arrow right under/below the 'question' icon

Yes, I agree. The issue also appears on the Order details page.

It would be nice to apply 'mobile' style to a tablet devices, I think. I mean, when a screen width is in [721-1050] px, the table becomes scrollable, transaction status is hidden. Seems to be a not convenient a bit, especially when we add 'Cancel transaction' feature to Explorer in the future.

Agree. We can change the limit and re-test.

I'd remove 'violet' color when I click on an order in a mobile view. The card is highlighted with violet when I press on a tooltip, or when I scroll the list and accidentally tough it.. Looks a bit strange to me, frankly.

In the same way as 2) Agree. Also it happen on the [Order details page](https://pr721--gpui.review.gnosisdev.com/rinkeby/orders/0x88537c74a03c324464d6c269d99b29e7289b105766c5368f77c76c241e1d5da8b6bad41ae76a11d10f7b0e664c5007b908bc77c9615b2dac/

@alfetopito , @alongoni , @biocom , WDYT?

Thanks

src/components/orders/OrdersUserDetailsTable/index.tsx

alongoni · 2021-10-05T17:53:20Z

@alongoni @biocom one extra question that came up is regarding the "invert price" button behaviour on mobile, and I'd like a UX input from you guys.

What do you think would be the best approach on mobile when clicked?

invert only the current order price

invert all table prices - like the desktop behaviour

If we think that the cards are like units and we invert the price, that will affect only the value within it. (1)
However, if we wish to have similar behavior as desktop (2), I believe a global option could be added between Order tab and the first card.

matextrem · 2021-10-05T20:20:40Z

Some notes from my side:

There is no pagination in the table: a user is able to see only 20 first orders

Not a priority, but it would be nice to show a tooltip arrow right under/below the 'question' icon

It would be nice to apply 'mobile' style to a tablet devices, I think. I mean, when a screen width is in [721-1050] px, the table becomes scrollable, transaction status is hidden. Seems to be a not convenient a bit, especially when we add 'Cancel transaction' feature to Explorer in the future.

I'd remove 'violet' color when I click on an order in a mobile view. The card is highlighted with violet when I press on a tooltip, or when I scroll the list and accidentally tough it.. Looks a bit strange to me, frankly.

@alfetopito , @alongoni , @biocom , WDYT?

Thanks

I think number 2 should be solved in another PR since it is not directly related to this issue

elena-zh · 2021-10-06T09:07:41Z

I was not able to reproduce the 2nd issue in an order details page, as tooltip arrow is always pointing to the question icon to me there

But anyways, I have created a separate issue for this case #722

alongoni · 2021-10-06T13:34:41Z

@matextrem We have a problem with the pagination and controls.

iOS:

💡 On mobile, maybe we can reduce the "Rows per page" font-size and margin between elements (Rows per page | page range | arrows).

Additionally, in some screens, when the "limit price" order is changed, a new line appears:
https://www.loom.com/share/948ec959328a48ec8b000b346e2d2c01

alfetopito · 2021-10-06T18:20:47Z

@matextrem We have a problem with the pagination and controls. iOS:

bulb On mobile, maybe we can reduce the "Rows per page" font-size and margin between elements (Rows per page | page range | arrows).

Additionally, in some screens, when the "limit price" order is changed, a new line appears: https://www.loom.com/share/948ec959328a48ec8b000b346e2d2c01

Would it be too little info if on mobile we don't show rows per page text at all?
AND/OR wrap the pagination controls in another line

Another question for @biocom

henrypalacios · 2021-10-06T19:40:27Z

Good job @matextrem, I agree with @alfetopito, maybe with just the 20▼ would be enough.

But maybe to avoid future problems the best is to throw it under the tabs. Is it possible with the CSS magic @alongoni?

Otherwise it occurs to me in another PR, send the extra content in header bar tabs to the main content and play with the visibility in mobile/desktop version.

I think @alongoni already mentioned these details I found.

matextrem · 2021-10-07T05:07:02Z

@henrypalacios I agree with the option of removing "rows per page" title on mobile to fix the pagination styling problem.

Regarding the second point, I did it this way in order to keep the separation between columns and wrap the text if necessary based on Michel comment
(Second bullet)

src/components/common/Tabs/Tabs.tsx

elena-zh · 2021-10-12T13:32:06Z

Changes LGTM.
However, still there is no decision how to invert prices in a mobile view.
A new nitpick: the section is jumping when invert a price.

SVID_20211012_163011_1.mp4

matextrem · 2021-10-12T13:41:52Z

Regarding the nitpick, that was a thing discussed along with @alongoni in order to keep separation between columns so I think the behavior is desired.

alfetopito

Good to go IMO

Let's keep the invert price behaviour as is implemented.

We'll revisit if there's feedback

alfetopito · 2021-10-12T17:19:57Z

src/components/orders/OrdersUserDetailsTable/index.tsx

          <RowWithCopyButton
            className="span-copybtn-wrap"
            textToCopy={uid}
            contentsToDisplay={
-              <Link to={`/orders/${order.uid}`} target="_blank">
+              <Link to={`/orders/${order.uid}`} rel="noopener noreferrer" target="_blank">


Not for this PR

Just realized would be good to use the same network component as with addresses.

Right now it works like this, but we unnecessarily query all networks before loading the order. But we already know the network it belongs to.

alfetopito · 2021-10-14T18:19:03Z

I've rebased against base branch and resolved conflicts

matextrem added the Protofire task to the protofire team label Oct 4, 2021

matextrem requested review from alfetopito, alongoni and henrypalacios October 4, 2021 22:07

matextrem self-assigned this Oct 4, 2021

alfetopito reviewed Oct 5, 2021

View reviewed changes

src/components/orders/OrdersUserDetailsTable/index.tsx Outdated Show resolved Hide resolved

src/components/orders/OrdersUserDetailsTable/index.tsx Outdated Show resolved Hide resolved

fairlighteth reviewed Oct 5, 2021

View reviewed changes

src/apps/explorer/components/common/ExplorerTabs/ExplorerTab.tsx Outdated Show resolved Hide resolved

fairlighteth reviewed Oct 5, 2021

View reviewed changes

src/components/orders/OrdersUserDetailsTable/index.tsx Outdated Show resolved Hide resolved

fairlighteth reviewed Oct 5, 2021

View reviewed changes

src/components/orders/OrdersUserDetailsTable/index.tsx Outdated Show resolved Hide resolved

fairlighteth reviewed Oct 5, 2021

View reviewed changes

src/components/orders/OrdersUserDetailsTable/index.tsx Show resolved Hide resolved

alongoni reviewed Oct 5, 2021

View reviewed changes

src/components/orders/OrdersUserDetailsTable/index.tsx Outdated Show resolved Hide resolved

matextrem requested review from alfetopito, alongoni and fairlighteth October 5, 2021 20:20

elena-zh mentioned this pull request Oct 6, 2021

Mobile: Show a tooltip arrow right under/below the 'question' icon #722

Closed

alongoni approved these changes Oct 12, 2021

View reviewed changes

alongoni reviewed Oct 12, 2021

View reviewed changes

src/components/common/Tabs/Tabs.tsx Show resolved Hide resolved

alfetopito approved these changes Oct 12, 2021

View reviewed changes

alfetopito mentioned this pull request Oct 12, 2021

Internal links should re-use network when possible #747

Closed

henrypalacios approved these changes Oct 13, 2021

View reviewed changes

henrypalacios changed the base branch from develop to release/2.3.0 October 14, 2021 14:37

matextrem added 4 commits October 14, 2021 11:00

Add responsive design for orders table

07e430e

Fix linter issues

4db39e6

Fix linting issues

352e016

Fix pagination styles

31536e3

alfetopito force-pushed the feature/693-responsive-table branch from a1a2918 to 31536e3 Compare October 14, 2021 18:18

alfetopito added the Auto-merge PRs with this tag will be automatically merged when approved and CI succeeds label Oct 14, 2021

mergify bot merged commit 6a8a331 into release/2.3.0 Oct 14, 2021

alfetopito deleted the feature/693-responsive-table branch October 14, 2021 18:26

alfetopito mentioned this pull request Oct 14, 2021

Release/2.3.0 #761

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Explorer] Orders table responsive #721

[Explorer] Orders table responsive #721

matextrem commented Oct 4, 2021 •

edited by henrypalacios

Loading

github-actions bot commented Oct 4, 2021

alfetopito left a comment

fairlighteth commented Oct 5, 2021

fairlighteth Oct 5, 2021

fairlighteth Oct 5, 2021

matextrem Oct 5, 2021

matextrem Oct 5, 2021

elena-zh commented Oct 5, 2021 •

edited

Loading

fairlighteth commented Oct 5, 2021

alfetopito commented Oct 5, 2021

alongoni commented Oct 5, 2021

alongoni commented Oct 5, 2021

matextrem commented Oct 5, 2021

elena-zh commented Oct 6, 2021

alongoni commented Oct 6, 2021 •

edited

Loading

alfetopito commented Oct 6, 2021

henrypalacios commented Oct 6, 2021

matextrem commented Oct 7, 2021

elena-zh commented Oct 12, 2021

matextrem commented Oct 12, 2021

alfetopito left a comment

alfetopito Oct 12, 2021

alfetopito commented Oct 14, 2021

[Explorer] Orders table responsive #721

[Explorer] Orders table responsive #721

Conversation

matextrem commented Oct 4, 2021 • edited by henrypalacios Loading

Summary

To Test

github-actions bot commented Oct 4, 2021

alfetopito left a comment

Choose a reason for hiding this comment

fairlighteth commented Oct 5, 2021

fairlighteth Oct 5, 2021

Choose a reason for hiding this comment

fairlighteth Oct 5, 2021

Choose a reason for hiding this comment

matextrem Oct 5, 2021

Choose a reason for hiding this comment

matextrem Oct 5, 2021

Choose a reason for hiding this comment

elena-zh commented Oct 5, 2021 • edited Loading

fairlighteth commented Oct 5, 2021

alfetopito commented Oct 5, 2021

alongoni commented Oct 5, 2021

alongoni commented Oct 5, 2021

matextrem commented Oct 5, 2021

elena-zh commented Oct 6, 2021

alongoni commented Oct 6, 2021 • edited Loading

alfetopito commented Oct 6, 2021

henrypalacios commented Oct 6, 2021

matextrem commented Oct 7, 2021

elena-zh commented Oct 12, 2021

matextrem commented Oct 12, 2021

alfetopito left a comment

Choose a reason for hiding this comment

alfetopito Oct 12, 2021

Choose a reason for hiding this comment

alfetopito commented Oct 14, 2021

matextrem commented Oct 4, 2021 •

edited by henrypalacios

Loading

elena-zh commented Oct 5, 2021 •

edited

Loading

alongoni commented Oct 6, 2021 •

edited

Loading