New layout for trade page #139

Vanvians · 2023-10-01T19:00:53Z

Closes #80
Implements new trade page layout

ui/page/root/trade_page.go

ui/values/localizable/en.go

ui/values/strings.go

ukane-philemon · 2023-10-02T06:59:44Z

It seems part of the icons are hidden, no margin-top effect for the title, and the grey box looks smaller than what's in the design. Testing on MacOS.

Sirmorrison

From the screenshot you added, it shows that your icons are larger than the design. Kindly drop them to Layout48dp or Layout36dp.

Also, i will like to recommend that you split the codes inside here pageContentLayout() into multiple sub functions. That way you have a function/method for all the components of the page, and you call these functions in the mother function.

Lastly, I will recommend you wrap pageContentLayout() in the components.UniformPadding(). These will help properly position the page contents especially when i full screen display.

ui/page/root/trade_page.go

Sirmorrison · 2023-10-02T06:53:42Z

ui/page/root/trade_page.go

+					layout.Rigid(func(gtx C) D {
+						return layout.Inset{Bottom: values.MarginPadding0}.Layout(gtx, pg.Theme.Label(values.TextSize20, values.String(values.StrExchangeIntro)).Layout)
+					}),
+					layout.Rigid(func(gtx C) D {
+						return layout.Inset{Bottom: values.MarginPadding16}.Layout(gtx, pg.Theme.Label(values.TextSize20, values.String(values.StrExchangeIntroPt2)).Layout)
+					}),


Why do we have exchange intro 1 and 2?

when it is one string I am unable get the second line to be exactly like the design.

Right.. use this to break the string into 2, then centralize.
text := values.StringF(values.StrSourceModalInfo,
). In your string variable, add %v where you want the string to break.

ui/page/root/trade_page.go

ukane-philemon

Uhmm, I missed these in my first review. Your changes look straightforward.

ui/page/root/trade_page.go

Sirmorrison

From figma..

your UI..

The Dex icon is still too big. it should be reduce by half the current size.
There should be space between the DCR DEX not DCRDEX.
The items in the boxes are not centralised.

ui/page/root/trade_page.go

Sirmorrison · 2023-10-09T05:43:41Z

ui/values/localizable/en.go

+"exchangeIntro" = "Exchange currency using our predefined/custom servers or through a centralized exchange,"
+"exchangeIntroPt2" = "it is simple, quick, and secure."


Suggested change

"exchangeIntro" = "Exchange currency using our predefined/custom servers or through a centralized exchange,"

"exchangeIntroPt2" = "it is simple, quick, and secure."

"exchangeIntro" = "Exchange currency using our predefined/custom servers or through a centralized exchange,%v it is simple, quick, and secure."

Sirmorrison · 2023-10-09T05:45:15Z

ui/page/root/trade_page.go

+						layout.Rigid(pg.titleContent(values.String(values.StrExchangeIntro))),
+						layout.Rigid(pg.titleContent(values.String(values.StrExchangeIntroPt2))),


Suggested change

layout.Rigid(pg.titleContent(values.String(values.StrExchangeIntro))),

layout.Rigid(pg.titleContent(values.String(values.StrExchangeIntroPt2))),

layout.Rigid(pg.titleContent(values.StringF(values.StrExchangeIntro, "<br/>"))),

Use this to break the string into 2, then centralize.
text := values.StringF(values.StrSourceModalInfo, "<br/>"). In your string variable, add %v where you want the string to break.
you don't need two strings.

This doesn't work, It breaks the sentence but doesn't allow the second line to move independently

ui/page/root/trade_page.go

ukane-philemon · 2023-10-09T22:22:15Z

There should be space between the DCR DEX not DCRDEX.

@Sirmorrison, I requested @Vanvians to not separate the word DCRDEX in a previous review. For example, see how it’s used here: https://dex.decred.org/.

Vanvians · 2023-10-09T22:59:24Z

How it looks with requested changes. Any other comments?

dreacot · 2023-10-11T09:59:23Z

@Sirmorrison @ukane-philemon @Vanvians

sorry for this setback but the layout of the trade tab has been updated as seen in the screenshot below

i have a PR for the segmented control here #175

Sirmorrison · 2023-10-11T10:44:10Z

IS this PR deprecated @dreacot ?

dreacot · 2023-10-11T10:48:42Z

It is, a new PR should be opened, and linked here before closing this

Vanvians force-pushed the new_trade_layout branch from 277b572 to 5f3c5a0 Compare October 2, 2023 04:10

Vanvians marked this pull request as draft October 2, 2023 04:21

Vanvians force-pushed the new_trade_layout branch from 1973180 to 347d1d1 Compare October 2, 2023 04:32

Vanvians marked this pull request as ready for review October 2, 2023 04:32

Vanvians requested review from ukane-philemon and Sirmorrison October 2, 2023 04:32

Vanvians force-pushed the new_trade_layout branch from 34a182a to 74df42c Compare October 2, 2023 05:22

ukane-philemon requested changes Oct 2, 2023

View reviewed changes

Sirmorrison suggested changes Oct 2, 2023

View reviewed changes

ukane-philemon requested changes Oct 2, 2023

View reviewed changes

ui/page/root/trade_page.go Outdated Show resolved Hide resolved

ui/page/root/trade_page.go Outdated Show resolved Hide resolved

dreacot requested review from ukane-philemon and Sirmorrison October 3, 2023 00:29

ukane-philemon requested changes Oct 4, 2023

View reviewed changes

ui/page/root/trade_page.go Outdated Show resolved Hide resolved

ui/page/root/trade_page.go Outdated Show resolved Hide resolved

Sirmorrison suggested changes Oct 9, 2023

View reviewed changes

Sirmorrison reviewed Oct 9, 2023

View reviewed changes

ui/page/root/trade_page.go Outdated Show resolved Hide resolved

Vanvians added 11 commits October 10, 2023 12:51

New layout for trade page

e88b021

update trade page layout

607e770

resolve conflicts

c37097d

cleanup code

3505fea

start trade page from while on fullscreen

574c5fa

Make requested changes

2bca6de

reviewed changes

443397c

New layout for trade page

b4a915e

resolve conflicts

9df4885

cleanup code

cddcddf

Make requested changes

93fea0e

Vanvians force-pushed the new_trade_layout branch from f8c5e2f to 93fea0e Compare October 10, 2023 18:58

fix icons

8040168

Vanvians force-pushed the new_trade_layout branch from e0d669d to 8040168 Compare October 10, 2023 19:09

Vanvians requested a review from ukane-philemon October 10, 2023 19:12

dreacot mentioned this pull request Oct 13, 2023

implement the new design for the landing page of the trade tab #80

Closed

dreacot closed this Oct 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New layout for trade page #139

New layout for trade page #139

Vanvians commented Oct 1, 2023 •

edited

Loading

ukane-philemon commented Oct 2, 2023 •

edited

Loading

Sirmorrison left a comment

Sirmorrison Oct 2, 2023

Vanvians Oct 2, 2023

Sirmorrison Oct 4, 2023

ukane-philemon left a comment

Sirmorrison left a comment

Sirmorrison Oct 9, 2023

Sirmorrison Oct 9, 2023

Vanvians Oct 9, 2023

ukane-philemon commented Oct 9, 2023

Vanvians commented Oct 9, 2023 •

edited

Loading

dreacot commented Oct 11, 2023

Sirmorrison commented Oct 11, 2023

dreacot commented Oct 11, 2023

		"exchangeIntro" = "Exchange currency using our predefined/custom servers or through a centralized exchange,"
		"exchangeIntroPt2" = "it is simple, quick, and secure."

		layout.Rigid(pg.titleContent(values.String(values.StrExchangeIntro))),
		layout.Rigid(pg.titleContent(values.String(values.StrExchangeIntroPt2))),

New layout for trade page #139

New layout for trade page #139

Conversation

Vanvians commented Oct 1, 2023 • edited Loading

ukane-philemon commented Oct 2, 2023 • edited Loading

Sirmorrison left a comment

Choose a reason for hiding this comment

Sirmorrison Oct 2, 2023

Choose a reason for hiding this comment

Vanvians Oct 2, 2023

Choose a reason for hiding this comment

Sirmorrison Oct 4, 2023

Choose a reason for hiding this comment

ukane-philemon left a comment

Choose a reason for hiding this comment

Sirmorrison left a comment

Choose a reason for hiding this comment

Sirmorrison Oct 9, 2023

Choose a reason for hiding this comment

Sirmorrison Oct 9, 2023

Choose a reason for hiding this comment

Vanvians Oct 9, 2023

Choose a reason for hiding this comment

ukane-philemon commented Oct 9, 2023

Vanvians commented Oct 9, 2023 • edited Loading

dreacot commented Oct 11, 2023

Sirmorrison commented Oct 11, 2023

dreacot commented Oct 11, 2023

Vanvians commented Oct 1, 2023 •

edited

Loading

ukane-philemon commented Oct 2, 2023 •

edited

Loading

Vanvians commented Oct 9, 2023 •

edited

Loading