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

Action button bug fixes #339

Merged
merged 3 commits into from
Sep 26, 2022
Merged

Action button bug fixes #339

merged 3 commits into from
Sep 26, 2022

Conversation

alse
Copy link
Contributor

@alse alse commented Sep 22, 2022

  • Disable action buttons when code editor is empty
  • Re-enable action buttons after errors and show error alert
    This is a temporary solution, we'll be switching to Apollo 3 and mutation hooks

For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the standards mentioned here.
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@vercel
Copy link

vercel bot commented Sep 22, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
flow-playground ✅ Ready (Inspect) Visit Preview Sep 26, 2022 at 9:19PM (UTC)

@alse alse force-pushed the as/action-button-bug-fixes branch from 12b96c8 to 75500a2 Compare September 22, 2022 15:22
@vercel vercel bot temporarily deployed to Preview September 22, 2022 15:24 Inactive
This is a temporary solution, we'll be switching to Apollo 3 and
mutation hooks
@alse alse force-pushed the as/action-button-bug-fixes branch from 75500a2 to 17c39b7 Compare September 22, 2022 17:16
@vercel vercel bot temporarily deployed to Preview September 22, 2022 17:18 Inactive
Copy link
Contributor

@MrDSGC MrDSGC left a comment

Choose a reason for hiding this comment

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

Just a question on how the code handles getActiveCode() possibly returning undefined then LGTM

const { isSavingCode } = useProject();
const sendingTransaction = false;

const code = getActiveCode()[0].trim();
Copy link
Contributor

Choose a reason for hiding this comment

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

will this fatal if getActiveCode() returns undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getActiveCode is typed as a [string, number] tuple so it seems like that shouldn't happen. However, since typescript assumes accessing an array always returns a value, it would technically be possible. I updated the default values, and we should consider setting noUncheckedIndexedAccess: true in the future

@vercel vercel bot temporarily deployed to Preview September 26, 2022 21:19 Inactive
@alse alse merged commit 48c33b5 into staging Sep 26, 2022
@alse alse deleted the as/action-button-bug-fixes branch September 26, 2022 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants