Multi: Create DEX main page #208

ukane-philemon · 2023-10-27T02:20:02Z

Closes #187

The new design(for the order form) in this PR has been agreed upon by @itswisdomagain, everything else stays the same.

Also, some notable UI elements have been omitted in this PR (e.g the order filter row).

ukane-philemon · 2023-10-27T02:40:08Z

Uhmm, I’ll have to do a little cleanup on this PR in the morning and will remove the temp code that allows testing the whole flow. Until then, reviewers can run this PR to view the implemented UI.

ui/page/dcrdex/dcrdex_page.go

ui/page/dcrdex/market.go

dreacot · 2023-10-30T22:05:55Z

resolve conflicts

dreacot · 2023-10-31T16:32:06Z

Also, some notable UI elements have been omitted in this PR (e.g the order filter row).

any particular reason for this?

dreacot

clicking the background should close the dropdowns

the buy sell buttons might need a bit more left right padding and it seems there's a misalignment with the limit dropdown

there seems to be a horizontal scroll for the open orders section on figma

ui/page/dcrdex/market.go

ukane-philemon · 2023-11-01T14:31:01Z

there seems to be a horizontal scroll for the open orders section on figma

The columns fit correctly, maybe we can do without the horizontal scroll.

ukane-philemon · 2023-11-01T14:32:00Z

Also, some notable UI elements have been omitted in this PR (e.g the order filter row).

any particular reason for this?

Yes, I wanted to have something up and ready for review.

dreacot · 2023-11-01T16:24:16Z

there seems to be a horizontal scroll for the open orders section on figma

The columns fit correctly, maybe we can do without the horizontal scroll.

i see a last column called status after the settled column

ui/cryptomaterial/dropdown.go

ui/cryptomaterial/segmented_control.go

ui/modal/info_modal.go

ui/page/dcrdex/dcrdex_page.go

ukane-philemon · 2023-11-02T18:37:56Z

clicking the background should close the dropdowns

Not fixed yet. I'm unable to fix the backdrop layout issue atm.

JustinBeBoy

@dreacot according figma we should use Market or Currency Pair ?
dropdown should close when click outside, I remember it was handled in previous versions

we don't have slide on this ?

ui/cryptomaterial/dropdown.go

ui/page/dcrdex/dex_onboarding_page.go

ui/page/dcrdex/market.go

ukane-philemon · 2023-11-03T10:17:28Z

@dreacot according figma we should use Market or Currency Pair ?

Darn, I missed this.

dropdown should close when click outside, I remember it was handled in previous versions

Yes but it’s buggy so I created an issue to discuss other options.

we don't have slide on this ?

I’m leaving out the slide and some other elements on this page for a follow up PR.

JustinBeBoy · 2023-11-04T08:41:10Z

Some display functions are still missing but as @ukane-philemon said it can be improved later. Otherwise everything is fine with me

ui/cryptomaterial/dropdown.go

ui/cryptomaterial/segmented_control.go

ui/page/dcrdex/dcrdex_page.go

ui/cryptomaterial/dropdown.go

ui/page/dcrdex/dex_onboarding_page.go

ui/page/governance/proposals_page.go

ui/page/dcrdex/market.go

itswisdomagain · 2023-11-14T02:27:28Z

ui/page/dcrdex/market.go

+			return pg.Theme.List(pg.scrollContainer).Layout(gtx, 1, func(gtx C, index int) D {
+				return layout.Stack{}.Layout(gtx,
+					layout.Expanded(func(gtx C) D {
+						return layout.Inset{Top: 110}.Layout(gtx, func(gtx C) D {
+							return pg.pageContainer.Layout(gtx, len(pageContent), func(gtx C, i int) D {
+								return pageContent[i](gtx)
+							})
+						})
+					}),
+					layout.Stacked(pg.serverAndCurrencySelection()),
+				)
+			})
+		})


Why wrap in a pg.Theme.List? It looks to me that this will produce a list inside a list; why not just one list holding everything?

If using a list inside a list is necessary, then instead of a stack layout with the first item having a margin top to accommodate the serverAndCurrencySelection widget, consider using a flex widget with the serverAndCurrencySelection wrapped in a layout.Rigid and the other widget wrapped in a layout.Expanded.

Summary: I think you just need one list holding pg.serverAndCurrencySelection, pg.priceAndVolumeDetail, pg.orderFormAndOrderBook, and pg.openOrdersAndHistory, with the proper spacing between them.

pg.serverAndCurrencySelection stacks on top of the rest of the page when it's expanded according to the design and the whole page should be scrollable.

If we have one List holding everything, pg.serverAndCurrencySelection will push the rest of the pages leaving a massive gap when expanded.

Please send links to the design you're referring to.

ui/page/dcrdex/market.go

Sirmorrison

First pass.

ui/cryptomaterial/dropdown.go

ui/page/dcrdex/dex_onboarding_page.go

ui/page/dcrdex/market.go

ui/page/transaction/transactions_page.go

ui/page/governance/proposals_page.go

ui/page/governance/consensus_page.go

ui/page/exchange/order_history_page.go

ui/cryptomaterial/dropdown.go

itswisdomagain · 2023-11-27T09:55:28Z

ui/page/dcrdex/market.go

+func (pg DEXMarketPage) addServerDisplay() func(gtx C) D {
+	return func(gtx cryptomaterial.C) cryptomaterial.D {


Actually, it can be selectable. When it is selected, you start the process of adding a server. If that process doesn't complete, you clear the selection. If it completes, you select the newly added server. The UI won't show the "Add Server" selection for long, if at all. If it does show for a brief moment, it's understandable. The user was supposed to select a server and he selected the option to add a new server. It makes sense if that selection is displayed briefly as long as the user's request to add a new server is honored.

itswisdomagain

I want to recommend moving the dropdown code changes to a new PR with screenshots for relevant portions of changed code, so we can clearly understand the intent of the code changes. I find the current code quite difficult to understand and that means they'd be difficult to maintain.

Alternatively, you can leave the code changes here but provide screenshots for different sections of changed code so the intention can be understood and some refactor or code comments can be suggested to improve the new code.

ui/cryptomaterial/dropdown.go

ukane-philemon · 2023-11-27T13:42:09Z

I want to recommend moving the dropdown code changes to a new PR with screenshots for relevant portions of changed code, so we can clearly understand the intent of the code changes

The modification to the dropdown component is only required by this page.

ukane-philemon · 2023-11-27T13:54:15Z

The UI won't show the "Add Server" selection for long, if at all.

The "Add Server" button will remain part of the dropdown even if the user is already connected to a DEX server and I don't think we'd want to make it "dropdown" item. See: #208 (comment)

EDIT: I made the extraDisplay a dropdown item.

itswisdomagain

Not much left holding this PR. Mostly the changes to the dropdown that are still unclear.

itswisdomagain · 2023-11-28T01:53:19Z

ui/cryptomaterial/dropdown.go

+	group                    uint
+	closeAllDropdown         func(group uint)
+	isDropdownGroupCollapsed func(group uint) bool
+	Width                    unit.Dp
+	linearLayout             *LinearLayout
+	padding                  layout.Inset
+	shadow                   *Shadow
+	expandedViewAlignment    layout.Direction
+
+	FontWeight                             font.Weight
+	BorderWidth                            unit.Dp
+	BorderColor                            *color.NRGBA
+	Background                             *color.NRGBA
+	SelectedItemIconColor                  *color.NRGBA
+	MakeSelectedItemHoverableAfterCollapse bool
+	SelectedItemDirectionAfterCollapse     layout.Direction
+	StackBelowCollapsedLayout              bool
+	ExpandedLayoutInset                    layout.Inset
+	collapsedLayoutInset                   layout.Inset


I'm still weary of the many new properties here, and I want to believe there is a simpler, easier-to-understand way to achieve what you want to achieve. Comment on each change and attach a screenshot of the UI that results from that change.

For example, what does StackBelowCollapsedLayout mean? I assume that CollapsedLayout mean the dropdown's layout when collapsed. What is being stacked below it?

The new interpretation of the dropdown when not open is collapsed. I’m trying to factor in the naming to help understand what it’ll look like.

As shown in the design, when expanded or open, the list of items should be stacked below the collapsed/closed view NOT on top of it as we’ve been doing, hence the naming. This new dropdown layout is just for the market page in this PR.

I've renamed most of these properties and added comments.

ui/page/components/components.go

ui/page/components/utils.go

itswisdomagain · 2023-11-28T01:58:37Z

ui/page/governance/proposals_page.go

+						if pg.statusDropDown.Reversed() {
+							pg.statusDropDown.ExpandedLayoutInset.Right = values.MarginPadding55
+						} else {
+							pg.statusDropDown.ExpandedLayoutInset.Left = values.MarginPadding55
+						}


You may want to give Dropdown PaddingStart and PaddingEnd variables, then create the inset as needed in the dropdown layout code.

dropdown.ExpandedLayoutInset is updated and used on the fly cuz it has different padding on mobile and desktop.

ui/page/dcrdex/market.go

itswisdomagain · 2023-11-28T02:15:39Z

ui/page/dcrdex/market.go

+	pg.orderTypesDropdown.FontWeight = font.SemiBold
+	pg.orderTypesDropdown.MakeSelectedItemHoverableAfterCollapse = false
+	pg.orderTypesDropdown.SelectedItemIconColor = &pg.Theme.Color.Primary
+	pg.orderTypesDropdown.ExpandedLayoutInset = layout.Inset{Top: values.MarginPadding30}


Why? Share a screenshot highlighting how this top spacing is rendered.

It ensures vertical alignment with the buySell btn since they are both stacked.

Why? Share a screenshot highlighting how this top spacing is rendered.

By default, layout.Stack stacks its children on top of each other. The last child is always at the top but the designs show the last child (which is the expanded view) below the collapsed layout and in another case (i.e the order type dropdown) the expanded view is just halfway below the collapsed layout.

NOTE: This extra top (margin) when expanded, eats space from the last stack child, hence the need to account for that with len(d.items) * item default height (dp50) + d.ExpandedLayoutInset.Top

ui/page/dcrdex/market.go

ukane-philemon · 2023-11-28T05:42:38Z

Not much left holding this PR. Mostly the changes to the dropdown that are still unclear.

Dropdown changes look huge cuz we are trying to reuse the same dropdown for the DEX market page which has an entirely different design and layout.

If it helps, I can create another component instead that will be used by only the DEX pages (onboarding and market page).

I can also create a smaller PR for the original Dropdown refactoring in this PR if they are still desirable.

Signed-off-by: Philemon Ukane <[email protected]>

resolved

ukane-philemon changed the title ~~Create DEX main page~~ Multi: Create DEX main page Oct 27, 2023

dreacot requested review from JustinBeBoy, crux25, itswisdomagain, Sirmorrison and JoeGruffins October 27, 2023 02:41

ukane-philemon force-pushed the dex-main-screen branch 2 times, most recently from f89d9a9 to b916364 Compare October 27, 2023 15:29

ukane-philemon commented Oct 27, 2023

View reviewed changes

ui/page/dcrdex/dcrdex_page.go Outdated Show resolved Hide resolved

dreacot requested changes Oct 30, 2023

View reviewed changes

ui/page/dcrdex/market.go Outdated Show resolved Hide resolved

ukane-philemon force-pushed the dex-main-screen branch from d7e60d5 to 7672c39 Compare October 30, 2023 22:15

dreacot requested changes Oct 31, 2023

View reviewed changes

ui/page/dcrdex/market.go Show resolved Hide resolved

itswisdomagain requested changes Nov 2, 2023

View reviewed changes

ukane-philemon force-pushed the dex-main-screen branch 3 times, most recently from 93882ce to 333a7bd Compare November 2, 2023 18:17

JustinBeBoy requested changes Nov 3, 2023

View reviewed changes

JustinBeBoy approved these changes Nov 4, 2023

View reviewed changes

itswisdomagain requested changes Nov 4, 2023

View reviewed changes

ukane-philemon mentioned this pull request Nov 4, 2023

Integrate DCRDEX for DEX onboarding page #238

Merged

ukane-philemon force-pushed the dex-main-screen branch from 0e68441 to 9936a46 Compare November 5, 2023 23:24

itswisdomagain requested changes Nov 14, 2023

View reviewed changes

ukane-philemon force-pushed the dex-main-screen branch 2 times, most recently from a221580 to 123adcf Compare November 14, 2023 21:26

ukane-philemon requested a review from itswisdomagain November 14, 2023 22:20

Sirmorrison suggested changes Nov 20, 2023

View reviewed changes

ukane-philemon force-pushed the dex-main-screen branch from 123adcf to 4f4d3ba Compare November 20, 2023 14:16

dreacot approved these changes Nov 23, 2023

View reviewed changes

Sirmorrison suggested changes Nov 23, 2023

View reviewed changes

ukane-philemon force-pushed the dex-main-screen branch from 715b7fc to 0f5868a Compare November 23, 2023 19:42

ukane-philemon requested review from Sirmorrison and JustinBeBoy November 23, 2023 19:43

itswisdomagain reviewed Nov 27, 2023

View reviewed changes

itswisdomagain requested changes Nov 27, 2023

View reviewed changes

ui/cryptomaterial/dropdown.go Outdated Show resolved Hide resolved

ui/cryptomaterial/dropdown.go Outdated Show resolved Hide resolved

ukane-philemon force-pushed the dex-main-screen branch from 0f5868a to b06849e Compare November 27, 2023 15:04

itswisdomagain previously requested changes Nov 28, 2023

View reviewed changes

ukane-philemon force-pushed the dex-main-screen branch from 713af53 to 768f347 Compare November 28, 2023 21:09

ukane-philemon requested a review from itswisdomagain November 28, 2023 21:09

ukane-philemon force-pushed the dex-main-screen branch from 768f347 to b7c1446 Compare December 5, 2023 13:46

dreacot approved these changes Dec 5, 2023

View reviewed changes

ukane-philemon added 2 commits December 5, 2023 20:20

create DEX main page

0d454d3

Signed-off-by: Philemon Ukane <[email protected]>

reviews, refactor andd improved UI changes

06e0586

Signed-off-by: Philemon Ukane <[email protected]>

ukane-philemon force-pushed the dex-main-screen branch from b7c1446 to 06e0586 Compare December 5, 2023 19:36

dreacot approved these changes Dec 5, 2023

View reviewed changes

dreacot merged commit ab695e8 into crypto-power:master Dec 5, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multi: Create DEX main page #208

Multi: Create DEX main page #208

ukane-philemon commented Oct 27, 2023 •

edited

Loading

ukane-philemon commented Oct 27, 2023

dreacot commented Oct 30, 2023

dreacot commented Oct 31, 2023

dreacot left a comment

ukane-philemon commented Nov 1, 2023

ukane-philemon commented Nov 1, 2023

dreacot commented Nov 1, 2023

ukane-philemon commented Nov 2, 2023

JustinBeBoy left a comment

ukane-philemon commented Nov 3, 2023 •

edited

Loading

JustinBeBoy commented Nov 4, 2023

itswisdomagain Nov 14, 2023

ukane-philemon Nov 14, 2023

itswisdomagain Nov 27, 2023

ukane-philemon Nov 27, 2023 •

edited

Loading

Sirmorrison left a comment

itswisdomagain Nov 27, 2023

itswisdomagain left a comment

ukane-philemon commented Nov 27, 2023

ukane-philemon commented Nov 27, 2023 •

edited

Loading

itswisdomagain left a comment

itswisdomagain Nov 28, 2023

ukane-philemon Nov 28, 2023

ukane-philemon Nov 28, 2023

itswisdomagain Nov 28, 2023

ukane-philemon Nov 28, 2023

itswisdomagain Nov 28, 2023

ukane-philemon Nov 28, 2023

ukane-philemon Nov 28, 2023

ukane-philemon commented Nov 28, 2023

		func (pg DEXMarketPage) addServerDisplay() func(gtx C) D {
		return func(gtx cryptomaterial.C) cryptomaterial.D {

Multi: Create DEX main page #208

Multi: Create DEX main page #208

Conversation

ukane-philemon commented Oct 27, 2023 • edited Loading

ukane-philemon commented Oct 27, 2023

dreacot commented Oct 30, 2023

dreacot commented Oct 31, 2023

dreacot left a comment

Choose a reason for hiding this comment

ukane-philemon commented Nov 1, 2023

ukane-philemon commented Nov 1, 2023

dreacot commented Nov 1, 2023

ukane-philemon commented Nov 2, 2023

JustinBeBoy left a comment

Choose a reason for hiding this comment

ukane-philemon commented Nov 3, 2023 • edited Loading

JustinBeBoy commented Nov 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ukane-philemon Nov 27, 2023 • edited Loading

Choose a reason for hiding this comment

Sirmorrison left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itswisdomagain left a comment

Choose a reason for hiding this comment

ukane-philemon commented Nov 27, 2023

ukane-philemon commented Nov 27, 2023 • edited Loading

itswisdomagain left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ukane-philemon commented Nov 28, 2023

ukane-philemon commented Oct 27, 2023 •

edited

Loading

ukane-philemon commented Nov 3, 2023 •

edited

Loading

ukane-philemon Nov 27, 2023 •

edited

Loading

ukane-philemon commented Nov 27, 2023 •

edited

Loading