fix(trade): last transactions design #15502

adderpositive · 2024-11-21T13:46:58Z

Description

fix alignment of payment status
fix alignment of payment methods and provider
fix transaction margin

Related Issue

Resolve #15501

Screenshots

Before

After

packages/suite/src/views/wallet/coinmarket/common/CoinmarketPaymentType.tsx

adamhavel · 2024-11-26T12:05:47Z

packages/suite/src/views/wallet/coinmarket/common/CoinmarketProviderInfo.tsx

                </>
+            ) : (
+                <Row margin={{ left: spacings.xl }}>


Why the margins on both Row and Text? Why not use the gap on the parent Row?

This margin is the same as the width of a missing icon. With a gap, it would not work. I guess.

And why not have it in Text without Row?

Like:

<Text margin={{ left: spacings.xxl }}>{exchange}</Text>

Yes, that's possible!

jvaclavik · 2024-11-28T11:12:33Z

packages/suite/src/views/wallet/coinmarket/common/CoinmarketIcon.tsx

+
+export const CoinmarketIcon = ({ iconUrl }: CoinmarketIconProps) => {
+    const currentTheme = useSelector(state => state.suite.settings.theme.variant);
+    const iconWrapperSize = iconSizes.mediumLarge + 4;


what is this magical number 4? :D Maybe it's easier to set iconWrapperSize manually like:

const ICON_WRAPPER_SIZE = 123

It is total padding in the wrapper. (2px left - mediumLarge size of image - 2px right)

OK, then I suggest to use it this way:

const iconWrapperSize = iconSizes.mediumLarge + 2 * SIDE_PADDING;

packages/suite/src/views/wallet/coinmarket/common/CoinmarketIcon.tsx

jvaclavik · 2024-11-28T11:19:49Z

...arket/common/CoinmarketTransactions/CoinmarketTransaction/CoinmarketTransactionContainer.tsx

            <Row flexWrap={isBelowDesktop ? 'wrap' : undefined}>
                <Column
-                    alignItems="flex-start"
+                    alignItems="stretch"


You can delete alignItems here. Yesterday we switched Column default value to normal which is basically the same like stretch.

jvaclavik · 2024-11-28T11:20:36Z

...coinmarket/common/CoinmarketTransactions/CoinmarketTransaction/CoinmarketTransactionInfo.tsx

@@ -27,8 +27,10 @@ export const CoinmarketTransactionInfo = ({ trade }: CoinmarketTransactionInfoPr

    return (
        <Text margin={{ top: spacings.xs }} variant="tertiary" typographyStyle="label" as="div">
-            {tradeType} • <FormattedDate value={date} date time /> •{' '}
-            <CoinmarketTransactionStatus trade={trade} />
+            <Row alignItems="center" flexWrap="wrap">


alignItems="center" for Row is the default value, so you can delete this prop

jvaclavik · 2024-11-28T11:21:15Z

...coinmarket/common/CoinmarketTransactions/CoinmarketTransaction/CoinmarketTransactionInfo.tsx

-            {tradeType} • <FormattedDate value={date} date time /> •{' '}
-            <CoinmarketTransactionStatus trade={trade} />
+            <Row alignItems="center" flexWrap="wrap">
+                {tradeType} • <FormattedDate value={date} date time /> •{' '}


Please use new InfoPair component here 🙏

See here: https://dev.suite.sldev.cz/components/develop/?path=/story/infopair--info-pair

It would be lovely to have it dynamic, not only to the left and right parts. :) But it is nice anyway.

We hear you :D

Please wait for merging this (should be soon) and refactor it to InfoSegments

It's merged

adderpositive · 2024-11-28T12:21:27Z

@jvaclavik fixes are in this commit 5138fd1. I needed to rebase to use features according to your feedback. Thanks for your review

jvaclavik

Two last comments and ready to merge 💪

jvaclavik · 2024-11-29T10:43:13Z

packages/suite/src/views/wallet/coinmarket/common/CoinmarketIcon.tsx

+    const SIDE_PADDING = 2;
+    const iconWrapperSize = iconSizes.mediumLarge + 2 * SIDE_PADDING;


Ideally these two could be extracted to top level. not to have it directly in CoinmarketIcon component (it's rerendering)

jvaclavik · 2024-11-29T10:44:06Z

packages/suite/src/views/wallet/coinmarket/common/CoinmarketIcon.tsx

+
+    return (
+        <CoinmarketIconWrapper $isDark={currentTheme === 'dark'}>
+            <Row


Why do you need Row here if there is only one element?

You can use new component Box for this use case. It's an universal building block with many styling props.

😁 updated in 83d97a7

tomasklim · 2024-12-01T22:40:21Z

/rebase

github-actions · 2024-12-01T22:40:36Z

Start rebasing: https://github.com/trezor/trezor-suite/actions/runs/12109678481

adderpositive requested a review from adamhavel November 21, 2024 13:46

tomasklim self-assigned this Nov 22, 2024

tomasklim reviewed Nov 23, 2024

View reviewed changes

packages/suite/src/views/wallet/coinmarket/common/CoinmarketPaymentType.tsx Outdated Show resolved Hide resolved

adderpositive requested a review from tomasklim November 25, 2024 13:38

adamhavel reviewed Nov 26, 2024

View reviewed changes

jvaclavik reviewed Nov 28, 2024

View reviewed changes

adderpositive self-assigned this Nov 28, 2024

adderpositive force-pushed the fix/last-transactions-design branch from 6c5c69f to 5138fd1 Compare November 28, 2024 12:20

adderpositive requested a review from jvaclavik November 28, 2024 12:20

adderpositive force-pushed the fix/last-transactions-design branch from 5138fd1 to 1b69294 Compare November 28, 2024 15:41

jvaclavik reviewed Nov 29, 2024

View reviewed changes

adderpositive force-pushed the fix/last-transactions-design branch from 1b69294 to 83d97a7 Compare November 29, 2024 12:47

tomasklim approved these changes Dec 1, 2024

View reviewed changes

fix(trade): last transactions design

f454923

trezor-ci force-pushed the fix/last-transactions-design branch from 83d97a7 to f454923 Compare December 1, 2024 22:41

tomasklim enabled auto-merge (rebase) December 1, 2024 22:41

tomasklim merged commit 10e5bc4 into develop Dec 1, 2024
21 checks passed

tomasklim deleted the fix/last-transactions-design branch December 1, 2024 22:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(trade): last transactions design #15502

fix(trade): last transactions design #15502

adderpositive commented Nov 21, 2024 •

edited

Loading

adamhavel Nov 26, 2024

adderpositive Nov 26, 2024

jvaclavik Nov 28, 2024

adderpositive Nov 28, 2024

jvaclavik Nov 28, 2024

adderpositive Nov 28, 2024

jvaclavik Nov 28, 2024

jvaclavik Nov 28, 2024

jvaclavik Nov 28, 2024

jvaclavik Nov 28, 2024

adderpositive Nov 28, 2024

jvaclavik Nov 28, 2024

jvaclavik Nov 28, 2024

adderpositive Nov 28, 2024

adderpositive commented Nov 28, 2024 •

edited

Loading

jvaclavik left a comment

jvaclavik Nov 29, 2024

jvaclavik Nov 29, 2024

adderpositive Nov 29, 2024

tomasklim commented Dec 1, 2024

github-actions bot commented Dec 1, 2024

		const SIDE_PADDING = 2;
		const iconWrapperSize = iconSizes.mediumLarge + 2 * SIDE_PADDING;

fix(trade): last transactions design #15502

fix(trade): last transactions design #15502

Conversation

adderpositive commented Nov 21, 2024 • edited Loading

Description

Related Issue

Screenshots

Before

After

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

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

adderpositive commented Nov 28, 2024 • edited Loading

jvaclavik 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

tomasklim commented Dec 1, 2024

github-actions bot commented Dec 1, 2024

adderpositive commented Nov 21, 2024 •

edited

Loading

adderpositive commented Nov 28, 2024 •

edited

Loading