Feat/add spinner to button #368

2023-02-27T15:48:35Z

lujakob commented

2023-02-27 15:48:35 +00:00

(Migrated from github.com)

Some information on the implementation:

I decided to use css style visibility to hide the button text, to keep the width of the button on changing the state to the loading spinner
I am actually used to handling the loading state outside the loading button, in the component where the button is used and the data handling of the button callback is handled. The loading state is then just passed down to the button. This way there is more control over the loading state, because it can be set and unset bevor and after the async request.

As it is right now, I think the loading is reset by a re-render of the button.

So I think in the long run, I think it makes sense to have the loading state handled in the parent component and pass it down to the button. I would need to refactor alls 4 places where the AsyncButton is used. What do you think?

Would it make sense to rename AsyncButton to LoaderButton or SpinnerButton?
Should I also apply AsyncButton to /settings/relays "Save" button?
I added a "disabled" property to the AsyncButton, for future use. Maybe a button should be disalbed, as long as form fields are not valid.

Some information on the implementation: * I decided to use css style visibility to hide the button text, to keep the width of the button on changing the state to the loading spinner * I am actually used to handling the loading state outside the loading button, in the component where the button is used and the data handling of the button callback is handled. The loading state is then just passed down to the button. This way there is more control over the loading state, because it can be set and unset bevor and after the async request. As it is right now, I think the loading is reset by a re-render of the button. So I think in the long run, I think it makes sense to have the loading state handled in the parent component and pass it down to the button. I would need to refactor alls 4 places where the AsyncButton is used. What do you think? * Would it make sense to rename AsyncButton to LoaderButton or SpinnerButton? * Should I also apply AsyncButton to /settings/relays "Save" button? * I added a "disabled" property to the AsyncButton, for future use. Maybe a button should be disalbed, as long as form fields are not valid.

v0l (Migrated from github.com) requested changes 2023-02-28 10:25:41 +00:00

v0l (Migrated from github.com) left a comment

I added a new Spinner for the fast zaps, can you use that one instead? Please rebase and you will see it

lujakob commented

2023-02-28 13:53:18 +00:00

(Migrated from github.com)

I added a new Spinner for the fast zaps, can you use that one instead? Please rebase and you will see it

The spinner is very opaque thought, because of global css styles for disabled buttons having opacity: 0.3 - this also affects the spinner. Please have a look, in case it might make sense to re-think buttons styles for the different states and maybe replace using opacity with changing color brightness for hover states.

> I added a new Spinner for the fast zaps, can you use that one instead? Please rebase and you will see it The spinner is very opaque thought, because of global css styles for disabled buttons having `opacity: 0.3` - this also affects the spinner. Please have a look, in case it might make sense to re-think buttons styles for the different states and maybe replace using `opacity` with changing color brightness for hover states.

v0l (Migrated from github.com) approved these changes 2023-02-28 14:16:36 +00:00

v0l (Migrated from github.com) left a comment

Will check styles once its merged, thanks

Sign in to join this conversation.

No reviewers

v0l