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

chore(trading): rename coinmarket to trading #16590

Merged
merged 6 commits into from
Jan 29, 2025
Merged

chore(trading): rename coinmarket to trading #16590

merged 6 commits into from
Jan 29, 2025

Conversation

adderpositive
Copy link
Contributor

Description

  • rename all components from Coinmarket prefix to Trading
  • rename all hooks from useCoinmarket prefix to useTrading
  • united part of necessary types with suite-common

Related Issue

Resolve #16543

@adderpositive adderpositive added +Invity Related to Invity project chore labels Jan 24, 2025
@adderpositive adderpositive self-assigned this Jan 24, 2025
Base automatically changed from chore/regional-move to chore/trading-api January 24, 2025 15:09
@adderpositive adderpositive force-pushed the chore/rename-coinmarket-to-trading branch from 0b7aa4b to fd14bbc Compare January 26, 2025 17:36
@adderpositive adderpositive force-pushed the chore/rename-coinmarket-to-trading branch 2 times, most recently from 9b38c9e to e2c4556 Compare January 27, 2025 10:08
@adderpositive adderpositive linked an issue Jan 27, 2025 that may be closed by this pull request
Rename Coinmarket to Trading in code #16543
Closed
@adderpositive adderpositive marked this pull request as ready for review January 27, 2025 10:10
@adderpositive adderpositive requested a review from vytick January 27, 2025 10:10
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 27, 2025 •
edited
Loading

🚀 Expo preview is ready!

  • Project → trezor-suite-preview
  • Platforms → android, ios
  • Scheme → trezorsuitelite
  • Runtime Version → 23
  • More info

Learn more about 𝝠 Expo Github Action

@adderpositive adderpositive requested a review from jbazant January 27, 2025 10:16
Base automatically changed from chore/trading-api to develop January 27, 2025 10:21
@adderpositive adderpositive force-pushed the chore/rename-coinmarket-to-trading branch from cf60a10 to 538a692 Compare January 27, 2025 10:23
adderpositive
adderpositive commented Jan 27, 2025
View reviewed changes
packages/suite-desktop-core/e2e/support/fixtures.ts Outdated
@@ -183,8 +183,8 @@ const test = base.extend<Fixtures>({
const recoveryPage = new RecoveryActions(page);
await use(recoveryPage);
},
marketPage: async ({ page, url }, use) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Vere-Grey is that correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

No it is not. It revers 3 days old PR #16516
Please remove this change

packages/suite-desktop-core/e2e/support/pageActions/marketActions.ts Outdated
@@ -259,9 +247,9 @@ export class MarketActions {
});
}

const redirecteTradeResponse = createRedirectedTradeResponse(this.url);
const redirectedTradeResponse = createRedirectedTradeResponse(this.page.url());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

And this @Vere-Grey is correct? Thank you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const redirectedTradeResponse = createRedirectedTradeResponse(this.page.url());
const redirectedTradeResponse = createRedirectedTradeResponse(this.url());

packages/suite/src/components/guide/Feedback.tsx Outdated Show resolved Hide resolved
packages/suite/src/hooks/wallet/trading/form/common/useTradingAccount.ts Outdated Show resolved Hide resolved
packages/suite/src/hooks/wallet/trading/form/useTradingSellForm.ts Outdated Show resolved Hide resolved
packages/suite/src/storage/migrations/trading/migrationCoinmarketToTrading.ts
@@ -0,0 +1,52 @@
import type { OnUpgradeFunc } from '@trezor/suite-storage';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tomasklim is that correct migration?

packages/suite/src/support/messages.ts
@@ -224,126 +224,126 @@ export default defineMessages({
defaultMessage:
'All fees included; the transaction fee is estimated at {feeAmount} ({feeAmountFiat}).',
},
TR_COINMARKET_SWAP_MODAL_FOR_YOUR_SAFETY: {
TR_TRADING_SWAP_MODAL_FOR_YOUR_SAFETY: {
Copy link
Contributor Author

@adderpositive adderpositive Jan 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@MiroslavProchazka can I change translations in this way?

Copy link
Member

Choose a reason for hiding this comment

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

translators will have to translate it again, is it ok? @MiroslavProchazka

packages/suite/src/views/wallet/tokens/common/TokensTable/TokenRow.tsx Outdated Show resolved Hide resolved
packages/suite/src/views/wallet/tokens/common/TokensTable/TokenRow.tsx Outdated Show resolved Hide resolved
packages/suite/src/views/wallet/tokens/common/TokensTable/TokenRow.tsx Outdated Show resolved Hide resolved
Vere-Grey
Vere-Grey requested changes Jan 27, 2025
View reviewed changes
Copy link
Contributor

@Vere-Grey Vere-Grey left a comment

Choose a reason for hiding this comment

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

There are 3 incorrectly resolved conflicts.

packages/suite-desktop-core/e2e/support/fixtures.ts Outdated
@@ -183,8 +183,8 @@ const test = base.extend<Fixtures>({
const recoveryPage = new RecoveryActions(page);
await use(recoveryPage);
},
marketPage: async ({ page, url }, use) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

No it is not. It revers 3 days old PR #16516
Please remove this change

packages/suite-desktop-core/e2e/support/pageActions/marketActions.ts Outdated Show resolved Hide resolved
packages/suite-desktop-core/e2e/support/pageActions/marketActions.ts Outdated
@@ -259,9 +247,9 @@ export class MarketActions {
});
}

const redirecteTradeResponse = createRedirectedTradeResponse(this.url);
const redirectedTradeResponse = createRedirectedTradeResponse(this.page.url());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const redirectedTradeResponse = createRedirectedTradeResponse(this.page.url());
const redirectedTradeResponse = createRedirectedTradeResponse(this.url());

@misartomas
Copy link

I have checked it on https://dev.suite.sldev.cz/suite-web/chore/rename-coinmarket-to-trading/web and all the main coinmarket (now trading) features should be working - checked also with real buy/sell/swap transactions. TEST OK.

@jvaclavik
Copy link
Contributor

Again?
image

@adderpositive adderpositive force-pushed the chore/rename-coinmarket-to-trading branch from 7b11f3b to 4ecc9df Compare January 28, 2025 15:16
@adderpositive adderpositive requested a review from a team as a code owner January 28, 2025 15:16
@adderpositive adderpositive force-pushed the chore/rename-coinmarket-to-trading branch from 4ecc9df to 57fa3ea Compare January 29, 2025 08:26
@tomasklim tomasklim enabled auto-merge (rebase) January 29, 2025 09:37
Vere-Grey
Vere-Grey approved these changes Jan 29, 2025
View reviewed changes
Copy link
Contributor

@Vere-Grey Vere-Grey 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-desktop-core/e2e/support/pageActions/marketActions.ts Outdated Show resolved Hide resolved
@tomasklim tomasklim merged commit 9fd0808 into develop Jan 29, 2025
34 of 35 checks passed
@tomasklim tomasklim deleted the chore/rename-coinmarket-to-trading branch January 29, 2025 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Vere-Grey Vere-Grey Vere-Grey approved these changes

@izmy izmy izmy approved these changes

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

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

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

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

@jvaclavik jvaclavik Awaiting requested review from jvaclavik

@adamhavel adamhavel Awaiting requested review from adamhavel

@vytick vytick Awaiting requested review from vytick

@jbazant jbazant Awaiting requested review from jbazant

@FreeWall FreeWall Awaiting requested review from FreeWall

@LukasRada LukasRada Awaiting requested review from LukasRada

Assignees

@adderpositive adderpositive

Labels
chore +Invity Related to Invity project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rename Coinmarket to Trading in code
6 participants
@adderpositive @misartomas @jvaclavik @izmy @tomasklim @Vere-Grey