Skip to content

[New TX Flow] Address copilot's suggestions#2014

Open
jeesunikim wants to merge 4 commits intofeature/new-tx-flowfrom
new-tx-flow-copilot
Open

[New TX Flow] Address copilot's suggestions#2014
jeesunikim wants to merge 4 commits intofeature/new-tx-flowfrom
new-tx-flow-copilot

Conversation

@jeesunikim
Copy link
Copy Markdown
Contributor

@jeesunikim jeesunikim commented Apr 7, 2026

I left a direct link to the comment in each updated file

@github-project-automation github-project-automation bot moved this to Backlog (Not Ready) in DevX Apr 7, 2026
/**
* Determines if the simulation result indicates a read-only transaction
* (no auth entries and no write footprint).
* (no write footprint).
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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


return (
<div
role="button"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

const className = `Tab ${addlClassName ?? ""}`;

if (t.href) {
if (t.href && !t.isDisabled) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

// directly in handleCustomSignAll.
useEffect(() => {
if (allSigned) {
if (allSigned && signMode === "individual") {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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


// Clear previous validated result so a failed re-validation can't leave
// stale validatedXdr enabling the next step.
setValidatedXdr(undefined);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

// signing + assembly, or after auto-assembly when no auth entries are
// present) so the sign step receives a transaction with
// simulation-derived resources/fees.
return !simulate.simulationResultJson || !simulate.assembledXdr;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

{ id: "record", label: "Record" },
{ id: "enforce", label: "Enforce" },
{ id: "record-allow-nonroot", label: "Record (allow non-root)" },
{ id: "record_allow_nonroot", label: "Record (allow non-root)" },
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

rpc takes in _ underscore #1936 (comment)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Might be good to add this as a comment since it breaks our convention.

@stellar-jenkins-ci
Copy link
Copy Markdown

@jeesunikim jeesunikim requested a review from quietbits April 7, 2026 23:30
@stellar-jenkins-ci
Copy link
Copy Markdown

@stellar-jenkins-ci
Copy link
Copy Markdown

Comment on lines +349 to +351
&quot;Record&quot; is recommended for simulation.
&quot;Record&quot; discovers which authorization entries are
required.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be better to escape quote characters than encode them like this. This was is hard to read and will harder to search for this text.

{ id: "record", label: "Record" },
{ id: "enforce", label: "Enforce" },
{ id: "record-allow-nonroot", label: "Record (allow non-root)" },
{ id: "record_allow_nonroot", label: "Record (allow non-root)" },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Might be good to add this as a comment since it breaks our convention.

variant="success"
variant={variant}
title="Your transaction envelope XDR is ready."
icon={<Icon.CheckCircle />}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the variant is dynamic, should the icon be dynamic as well?

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

Labels

None yet

Projects

Status: Backlog (Not Ready)

Development

Successfully merging this pull request may close these issues.

2 participants