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

Utils #131

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

Conversation

Akashdeep-Patra
Copy link
Contributor

constants and utils package added, a wrapper for axios request added

Copy link
Contributor

@swarajpure swarajpure left a comment

Choose a reason for hiding this comment

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

I guess it'd be better to have default value for method and responseType as well since most of the requests on the frontend will be GET and response type of most requests would be json only

What do you think @Akashdeep-Patra? 🤔

@Akashdeep-Patra
Copy link
Contributor Author

@swarajpure axios already has a default value for method i.e get, and i can put a json default for responseType?

@harshith-venkatesh
Copy link
Contributor

Hi @Akashdeep-Patra , I had a small request, There was a bad user experience observed when a user tries to perform send/receive operation multiple times due to declaring the value of receiver and currency type as null on close modal.
image

@Kratika0907 @swarajpure I have requested @Akashdeep-Patra in the call to make this change and include in this PR.

Thanks @Akashdeep-Patra for the quick fix

@@ -12,8 +12,6 @@ const TransactionOperation = (props) => {

const closeModal = () => {
showModal((prev) => !prev);
setReceiver('');
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the quick fix

components/transaction-operation/index.js Show resolved Hide resolved
timeout: 1000,
});

export const makeRequest = (obj) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

proper naming obj can be renamed as requestConfig

Copy link
Contributor Author

Choose a reason for hiding this comment

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

naming requestObject because it will also have other info i.e data, and config is a bit misleading

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