Skip to content

Conversation

@gcwioro
Copy link
Contributor

@gcwioro gcwioro commented Jun 24, 2024

This PR introduces changes to disable UI buttons temporarily to prevent users from sending multiple requests unintentionally. Specifically, the changes address an issue where users could trigger the same action multiple times by repeatedly clicking a button before the initial request is completed.

The issue was causing multiple identical requests to be sent, leading to redundant server calls and potential data processing errors.

@gcwioro gcwioro requested a review from thomas-burko June 24, 2024 08:47
"pinia": "^2.1.7",
"vee-validate": "^4.12.4",
"vue": "^3.4.13",
"vite-plugin-pages": "^0.32.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this dependency be removed? The plugin itself does not seem to be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it because, when I ran 'npm typecheck', I got an error stating that vite-plugin-pages was not found.

Copy link
Contributor

Choose a reason for hiding this comment

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

Its because vite-plugin-pages/client in

"types": ["vite/client", "vite-plugin-pages/client", "unplugin-icons/types/vue", "@fuse-open/types"],
can be removed. That should not be there anymore since we are not using file based routing anymore. Could you remove that one.

animation: dotSpin var(--duration) linear calc(var(--delay) * 0.155s) infinite alternate;
}
@keyframes dotSpin {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think its fine to keep the animation part of the component. You could however extend the tailwind configuration and introduce this specific animation if we would reuse it somewhere else.

@@ -0,0 +1,57 @@
<template>
<div v-if="isLoading">
<div class="loader">
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to use the utility classes from tailwind to keep the css consistent.

You can rewrite it with
inline-grid grid-flow-col w-full place-content-center justify-self-center

Copy link
Contributor

Choose a reason for hiding this comment

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

I would create a separate "component" for it and put it into primitives directly instead of making it part of the button. I think it can be reused not only with button.

Günther Cwioro and others added 2 commits February 20, 2025 11:41
@K-Dud K-Dud merged commit daee702 into main Feb 20, 2025
3 checks passed
@K-Dud K-Dud deleted the bugfix/disable-buttons-to-prevent-multiple-requests branch February 20, 2025 12:11
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.

4 participants