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

Button loading prop should not disable button #392

Closed
areddon opened this issue Feb 20, 2020 · 8 comments · Fixed by #495
Closed

Button loading prop should not disable button #392

areddon opened this issue Feb 20, 2020 · 8 comments · Fixed by #495
Assignees
Labels

Comments

@areddon
Copy link
Contributor

areddon commented Feb 20, 2020

Request

The loading prop on the Button component disables the button. I don't believe that this should be the default behavior. Let the consumer decide whether to disable the button or not. If they already pass in a prop to set loading, they can use the same one to disable if they want.

Not sure if it is a bug or desired behavior, but when setting loading to true, the button changes to a ghost button, regardless of initial type.

@jendowns
Copy link
Contributor

@areddon I think it would be less about flexibility and more about UX. If a button is loading, it's essentially in a "skeleton" state right?

@areddon
Copy link
Contributor Author

areddon commented Feb 20, 2020

@jendowns I think UX could solve it, regardless I do not think it should disable it by default.

For our use case. We render a Button that is used to expand/collapse a FilterPanel.
On initial load of the page, there is no data to show in the FilterPanel, so it is collapsed, and the Button has loading and disabled set to true.

Once the data loads, both loading and disabled are set to false so the user can click the Button expand the FilterPanel. When the user makes a selection in the panel, we send a request to fetch the updated counts, so the FilterPanel itself is in a loading state, but still expanded and usable. We don't want to make the whole FilterPanel a skeleton and changing all the counts to spinners/skeletons is too much visual flare.

To indicate to the user that the FilterPanel is loading updated data, we set the Button's loading prop to true, but we do not want to disable it, so they can still collapse it. We use the Button to keep consistency with when the user first visits the page.

Is the type changing a defect or expected?

@jendowns
Copy link
Contributor

jendowns commented Feb 20, 2020

Is the type changing a defect or expected?

Yeah it was purposely done.

I was curious in the scenario you outlined above...

  • If you want the button still clickable, why not have a loading state inside the open panel area?
  • If you keep the loading button, why would it be clickable/interactive?

Seems like you want to show the user that something is loading but also give them a chance to kinda, sorta, halfway interact with it? 🤔 Why allow the interaction at all? If it's not loaded, why would they even be able to open the panel at all?

@areddon
Copy link
Contributor Author

areddon commented Feb 20, 2020

If you want the button still clickable, why not have a loading state inside the open panel area?

We need to be able to indicate to the user that content is loading even when the FilterPanel is collapsed. This occurs on initial load and when filters are applied by means other than the filter panel.

If you keep the loading button, why would it be clickable/interactive?

They might add a filter via the FilterPanel and then decide to collapse while it is updating the counts. Its also possible that they add a filter from outside the FilterPanel and then want to expand it.

Seems like you want to show the user that something is loading but also give them a chance to kinda, sorta, halfway interact with it?

They should still be able to expand/collapse the FilterPanel regardless of whether it is fetching updated counts. The FilterPanel itself still remains interactive as well. I can add a single filter and then a second, before the updated counts return from the first.

@jendowns
Copy link
Contributor

jendowns commented Feb 20, 2020

Thanks @areddon -- is this coming from a design you received? I'm just wondering about some of the interactions you are describing above. I've never seen a design that has something partially loaded that allows interaction (to clarify, when it is a true "skeleton" type state or has a loading indicator).

@jendowns
Copy link
Contributor

Just spoke to Alex in Slack about this & saw a demo of the interactions he's referring to. Given his situation, I think it makes sense to go along with this & allow folks to toggle off disabled state even when loading={true} 👍

@SimonFinney
Copy link
Contributor

🎉 This issue has been resolved in version 1.22.0-prerelease.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@SimonFinney
Copy link
Contributor

🎉 This issue has been resolved in version 1.22.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants