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.

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

[ENG-3314] feat: support repeat minter #687

Conversation

teebszet
Copy link
Member

@teebszet teebszet commented Nov 28, 2023

🔘 PR Type

  • Enhancement

📜 Background

depends on: secretkeylabs/sats-connect#50

issue: https://linear.app/xverseapp/issue/ENG-3314

🔄 Changes

feat: support new sats-connect version

Impact:

  • sats-connect new function
  • create inscription screen UI changes

🖼 Screenshot / 📹 Video

Include screenshots or a video demonstrating the changes. This is especially helpful for UI changes.

✅ Review checklist

Please ensure the following are true before merging:

  • Code Style is consistent with the project guidelines.
  • Code is readable and well-commented.
  • No unnecessary or debugging code has been added.
  • Security considerations have been taken into account.
  • The change has been manually tested and works as expected.
  • Breaking changes and their impacts have been considered and documented.
  • Code does not introduce new technical debt or issues.

@teebszet teebszet changed the title feat: support create repeat inscriptions feat: ENG-3314 Nov 28, 2023
@teebszet teebszet requested a review from jordankzf November 28, 2023 13:56
@teebszet teebszet marked this pull request as ready for review November 28, 2023 14:24
Copy link
Contributor

@jordankzf jordankzf left a comment

Choose a reason for hiding this comment

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

I'm not familiar enough with this repo and my nits might slow things down, feel free to ignore them.

I hope I can contribute with manual testing instead.

package.json Outdated Show resolved Hide resolved
@@ -151,7 +154,8 @@ function ContentIcon({ type, content, contentType: inputContentType }: Props) {
{t('BRC20.TITLE')} {t(`BRC20.${brc20Details.op.toUpperCase()}`)}
</div>
<Suffix>
{brc20Details.tick} - {brc20Details.value}
{formatNumber(new BigNumber(brc20Details.value).multipliedBy(repeat).toString())}{' '}
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit)

Can we break this down?

src/app/screens/createInscription/index.tsx Outdated Show resolved Hide resolved
@teebszet teebszet changed the title feat: ENG-3314 [ENG-3314] feat: support repeat minter Nov 30, 2023
Copy link

border-radius: 40px;
`;

function FeeRow({
Copy link
Member

Choose a reason for hiding this comment

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

nit: we should move this component to a separate file

@teebszet teebszet merged commit 3515fbd into develop Nov 30, 2023
2 checks passed
@teebszet teebszet deleted the tim/eng-3314-repeat-inscriptions-extension-review-sign-broadcast-repeat branch November 30, 2023 15:03
@teebszet teebszet mentioned this pull request Dec 1, 2023
This was referenced Dec 7, 2023
teebszet added a commit that referenced this pull request Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants