varunKT001 / tomper-wear-ecommerce

E-commerce web-application for selling clothing essentials 😀
https://tomper-wear.netlify.app
MIT License
38 stars 48 forks source link

Loading spinner when button is disabled #36

Closed sagnik2001 closed 2 years ago

sagnik2001 commented 2 years ago

Issue reference: Loading spinner when the button is disabled

Proposed changes: Added spinner when an API request is made or when the button is disabled.

Type of change:

Checklist:

Additional info (if any):

I have made the changes in the components which I found if any left out I will do the desire changes

netlify[bot] commented 2 years ago

‼️ Deploy request for tomper-wear rejected. Learn more about Netlify's sensitive variable policy

🔨 Explore the source changes: 81179328adf605b6c007d830d503706a9de1242a

varunKT001 commented 2 years ago

@sagnik2001 There is nothing wrong with your implementation, but it creates some issues:

What I wanted you to do, was to create an src/components/Button component that will accept all the props:

const Button = ({ disabled, children, ...rest }) => {
  return (
    <button disabled={disabled} {...rest}>
      {disabled ? <Icon /> : children}
    </button>
  );
};

something like this ⬆️

After doing so, you can replace all the instances of <button> with this component <Button> where you can give all the props just like you did in the normal button. You also won't need to write any extra styles for that, you give the class name as you normally do. The point here is, not to make a check for loading in every component. You just create a separate button component and make all the checks inside that.

For the label, you can make a check manually

<label>
  { loading ? <Icon/> : 'text' }
</label>

I hope you got my point 👍

sagnik2001 commented 2 years ago

Okay,making the changes

sagnik2001 commented 2 years ago

Hey @varunKT001, check if this is what you want??

varunKT001 commented 2 years ago

@sagnik2001 Yeah, that's what I wanted, but you have to do some more changes 😅

<label>upload image</label>
<svg style={{ ... , width: '1.025rem' }}>
  ...
</svg>

For now, do these changes 👍

sagnik2001 commented 2 years ago

Check it @varunKT001, is it okay now??

varunKT001 commented 2 years ago

In the loading icon component, add a width of 1.025rem (I tested it, works fine)

Remove the width and height, otherwise, there is no sense of putting width: '1.025rem' image remove these two lines ⬆️

Use normal buttons for the google sign in/sign up (there is an issue with that, we'll into it later)

You didn't do this change. image Make these normal buttons <button>, in both the login and register pages.

also make sure to always check the console warnings before making a commit

Still, there is a warning image Remove the import Icon, since we are not using it.

sagnik2001 commented 2 years ago

Hey @varunKT00,check it hope it's fine now

varunKT001 commented 2 years ago

Looks good now 👍

Thanks for contributing @sagnik2001 🚀