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.

Sign up for GitHub

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

Jump to bottom

feat: UI facelift of the Affected transactions in the RBF flow #16569

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat: UI facelift of the Affected transactions in the RBF flow #16569

wants to merge 1 commit into from
+407 −428

Conversation

peter-sanderson
Copy link
Contributor

@peter-sanderson peter-sanderson commented Jan 23, 2025 •
edited
Loading

@peter-sanderson peter-sanderson force-pushed the bump_fee_affected_tx_refactoring branch 6 times, most recently from 04131d3 to a09e208 Compare January 27, 2025 15:03
@peter-sanderson peter-sanderson marked this pull request as ready for review January 27, 2025 15:03
peter-sanderson
peter-sanderson commented Jan 27, 2025
View reviewed changes
...e/modals/ReduxModal/UserContextModal/TxDetailModal/AffectedTransactions/DecreasedOutputs.tsx

import { AddressReplaceMeAfterAdamsMerge } from './AddressReplaceMeAfterAdamsMerge';

// Todo: reolace with Albinas component once its merged
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Waits for this PR: #16619

peter-sanderson
peter-sanderson commented Jan 27, 2025
View reviewed changes
...odal/UserContextModal/TxDetailModal/AffectedTransactions/AddressReplaceMeAfterAdamsMerge.tsx
@@ -0,0 +1,33 @@
import styled from 'styled-components';

// Todo: replace once Adams PR is merged (here https://github.com/trezor/trezor-suite/pull/16465 but shall be separated ane generalized first)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Waits for this PR to conclude: #16465

adamhavel
adamhavel requested changes Jan 27, 2025
View reviewed changes
...s/ReduxModal/UserContextModal/TxDetailModal/AffectedTransactions/AffectedTransactionItem.tsx Outdated Show resolved Hide resolved
...s/ReduxModal/UserContextModal/TxDetailModal/AffectedTransactions/AffectedTransactionItem.tsx Outdated Show resolved Hide resolved
...e/modals/ReduxModal/UserContextModal/TxDetailModal/AffectedTransactions/DecreasedOutputs.tsx Outdated Show resolved Hide resolved
...e/modals/ReduxModal/UserContextModal/TxDetailModal/AffectedTransactions/DecreasedOutputs.tsx Outdated Show resolved Hide resolved
...e/modals/ReduxModal/UserContextModal/TxDetailModal/AffectedTransactions/DecreasedOutputs.tsx Outdated Show resolved Hide resolved
...e/modals/ReduxModal/UserContextModal/TxDetailModal/AffectedTransactions/DecreasedOutputs.tsx Outdated Show resolved Hide resolved
@peter-sanderson peter-sanderson force-pushed the bump_fee_affected_tx_refactoring branch from a09e208 to a7ffca5 Compare January 27, 2025 16:22
@peter-sanderson peter-sanderson force-pushed the bump_fee_affected_tx_refactoring branch from a7ffca5 to 54371f8 Compare January 27, 2025 16:30
@peter-sanderson peter-sanderson force-pushed the bump_fee_affected_tx_refactoring branch from 54371f8 to a50ae55 Compare January 27, 2025 16:39
@peter-sanderson peter-sanderson force-pushed the bump_fee_affected_tx_refactoring branch from a50ae55 to 749b6fb Compare January 28, 2025 08:54
@peter-sanderson peter-sanderson mentioned this pull request Jan 28, 2025
Draft
jvaclavik
jvaclavik approved these changes Jan 28, 2025
View reviewed changes
Copy link
Contributor

@jvaclavik jvaclavik left a comment

Choose a reason for hiding this comment

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

LGTM!

packages/suite/src/support/messages.ts
@@ -6379,6 +6383,10 @@ export default defineMessages({
id: 'TR_BUMP_FEE',
defaultMessage: 'Bump fee',
},
TR_BUMP_FEE_SUBTEXT: {
id: 'TR_BUMP_FEE_SUBTEXT',
defaultMessage: 'Speed up this transaction confirmation by paying a higher fee. ',
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: remove space in the end?

...odal/UserContextModal/TxDetailModal/AffectedTransactions/AddressReplaceMeAfterAdamsMerge.tsx
};

// Todo: replace once Adams PR is merged (here https://github.com/trezor/trezor-suite/pull/16465 but shall be separated ane generalized first)
export const AddressReplaceMeAfterAdamsMerge = ({ value, isTruncated }: AddressProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

love the name :D

peter-sanderson
peter-sanderson commented Jan 29, 2025
View reviewed changes
...dals/ReduxModal/UserContextModal/TxDetailModal/AffectedTransactions/AffectedTransactions.tsx
<Translation id="TR_AFFECTED_TXS" />
</Banner>
<Table>
{chainedTxs.own.map(tx => (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • todo: eror with missing tbody

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamhavel adamhavel adamhavel approved these changes

@jvaclavik jvaclavik jvaclavik approved these changes

@MiroslavProchazka MiroslavProchazka Awaiting requested review from MiroslavProchazka MiroslavProchazka is a code owner

@karliatto karliatto Awaiting requested review from karliatto karliatto is a code owner

@marekrjpolak marekrjpolak Awaiting requested review from marekrjpolak marekrjpolak is a code owner

@enjojoy enjojoy Awaiting requested review from enjojoy

Assignees
No one assigned
Labels
None yet
Projects
Suite Desktop
Status: 🤝 Needs QA
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@peter-sanderson @jvaclavik @adamhavel