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

feat(feedback): Disable Feedback submit & cancel buttons while submitting #15408

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

ryan953
Copy link
Member

@ryan953 ryan953 commented Feb 13, 2025

While submitting feedback the Submit and Cancel buttons previously remained active. So you could smash that submit many times and submit many duplicate feedbacks.

This disabled the buttons while the network request is being fired off.

The disabled buttons are slightly grayed out by the browser. So it depends on what custom color you've picked:
SCR-20250213-kmpm

@ryan953 ryan953 requested a review from a team as a code owner February 13, 2025 19:37
Copy link
Contributor

github-actions bot commented Feb 13, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 22.95 KB - -
@sentry/browser - with treeshaking flags 22.73 KB - -
@sentry/browser (incl. Tracing) 35.88 KB - -
@sentry/browser (incl. Tracing, Replay) 72.88 KB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 66.38 KB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 77.13 KB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 90.09 KB +0.05% +43 B 🔺
@sentry/browser (incl. Feedback) 40.1 KB +0.1% +41 B 🔺
@sentry/browser (incl. sendFeedback) 27.58 KB - -
@sentry/browser (incl. FeedbackAsync) 32.38 KB - -
@sentry/react 24.78 KB - -
@sentry/react (incl. Tracing) 37.78 KB - -
@sentry/vue 27.14 KB - -
@sentry/vue (incl. Tracing) 37.59 KB - -
@sentry/svelte 22.99 KB - -
CDN Bundle 24.17 KB - -
CDN Bundle (incl. Tracing) 35.92 KB - -
CDN Bundle (incl. Tracing, Replay) 70.75 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 75.91 KB - -
CDN Bundle - uncompressed 70.64 KB - -
CDN Bundle (incl. Tracing) - uncompressed 106.73 KB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 217.95 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 230.51 KB - -
@sentry/nextjs (client) 38.75 KB - -
@sentry/sveltekit (client) 36.32 KB - -
@sentry/node 127.61 KB - -
@sentry/node - without tracing 97.9 KB - -
@sentry/aws-serverless 107.34 KB - -

View base workflow run

@ryan953
Copy link
Member Author

ryan953 commented Feb 13, 2025

actually, i'm going to see if i can easily disable the whole form, or all the controls inside it instead of only 2 buttons.

Comment on lines 123 to 124
await waitForSec(5);

Copy link
Member

Choose a reason for hiding this comment

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

What's this for?

Copy link
Member Author

Choose a reason for hiding this comment

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

d'oh

for testing

Copy link
Member Author

Choose a reason for hiding this comment

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

speedup loop

@@ -146,7 +148,7 @@ export function Form({
<ScreenshotInputComponent onError={onScreenshotError} />
) : null}

<div class="form__right" data-sentry-feedback={true}>
<fieldset class="form__right" data-sentry-feedback={true} disabled={isSubmitting}>
Copy link
Contributor

Choose a reason for hiding this comment

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

what does fieldset do?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's a container for form controls. it's got some default padding&margin and a border too.

the reason that i brought it in is because it supports disabled which will prevent the text boxes from accepting input.
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/fieldset#attributes

It's not applying to our <button> inside the form tho, so i had to do those manually. I suspect, but didn't test, that it would automatically disable <input type="button"> or ` if we were using those instead. But i don't wanna change it all over.

@ryan953 ryan953 merged commit 50f89f1 into develop Feb 13, 2025
113 checks passed
@ryan953 ryan953 deleted the ryan953/feedback-button-disable-while-loading branch February 13, 2025 23:59
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.

3 participants