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

Fix displaying texts in Loading in case of arguments are not passed #8261

Merged
merged 1 commit into from
Oct 17, 2022

Conversation

alioguzhan
Copy link
Contributor

Current behavior:

If we omit the loadingPrimary and loadingSecondary props, the component still displays the translation keys (ra.page.loading and ra.message.loading) beneath the spinner.

This fixes that.

@fzaninotto fzaninotto merged commit e7df406 into marmelab:master Oct 17, 2022
@fzaninotto fzaninotto added this to the 4.4.2 milestone Oct 17, 2022
@fzaninotto
Copy link
Member

Thanks!

@WiXSL
Copy link
Contributor

WiXSL commented Oct 17, 2022

@alioguzhan we made a mistake here.
Your changes make the component not use the translation function anymore.
Actually, the problem you are trying to solve is the expected behavior.
You need to pass an empty string in loadingPrimary and loadingSecondary in order to not see the default messages

@alioguzhan
Copy link
Contributor Author

Hmm, I see. I can revert the changes, No problem. But, Isn't it weird that we have to pass empty strings to hide the messages? Maybe we can find another solution? Like passing a flag hidePrimaryMessage and hideSecondaryMessage ? Or an options flag. Examples:

1:

<Loading hidePrimary hideSecondary />

2:

<Loading options={{ hide: { primary: true, secondary: false} }} /> 

We can find an alternative better than:

<Loading loadingPrimary="" loadingSecondary="" />

What do you think? @WiXSL

@WiXSL
Copy link
Contributor

WiXSL commented Oct 17, 2022

We've already reverted them.
We would like to keep react-admin components as simple as possible.
If the Loading component doesn't fit your needs, and you don't want to show any messages, you can use for example a MUI CircularProgress based component like the Loading component does.

@alioguzhan
Copy link
Contributor Author

Fair enough. Thanks for your time! @WiXSL

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

Successfully merging this pull request may close these issues.

3 participants