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

[Docs]: Improve buttons sanitation explanation. #4621

Merged
merged 3 commits into from
Apr 7, 2020

Conversation

WiXSL
Copy link
Contributor

@WiXSL WiXSL commented Apr 1, 2020

Because of #4574

Copy link
Contributor

@djhi djhi left a comment

Choose a reason for hiding this comment

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

👍

@djhi djhi added this to the 3.4.0 milestone Apr 2, 2020
@fzaninotto
Copy link
Member

This conflicts with #4544 (already in master). Let me merge master into next and you'll change what needs to be changed.

@fzaninotto
Copy link
Member

Done, please rebase

@fzaninotto fzaninotto removed this from the 3.4.0 milestone Apr 3, 2020
@WiXSL WiXSL force-pushed the fix-docs-button-sanitation branch from 79eef9c to 7b12716 Compare April 3, 2020 13:18
@WiXSL WiXSL changed the base branch from next to master April 3, 2020 13:19
@WiXSL
Copy link
Contributor Author

WiXSL commented Apr 3, 2020

Done.

@WiXSL WiXSL requested a review from djhi April 6, 2020 15:22
@djhi djhi modified the milestones: 3.5.0, 3.4.1 Apr 6, 2020
@fzaninotto
Copy link
Member

I don't understand, the previous code snippet already took care of sanitizing the extra props.

@Kmaschta
Copy link
Contributor

Kmaschta commented Apr 7, 2020

Since we fixed the CreateButton props sanitation, and this Create and Edit views documentation already have a lot of tips and examples, I'm in favor of removing this tip.
It is not the purpose of React Admin to explain how Material UI components work ?

Also, this is an example of heavy customation, and the Create / Edit views documentation isn't really the good place for that. We have plenty of examples of customation on the demo IMHO

@WiXSL
Copy link
Contributor Author

WiXSL commented Apr 7, 2020

Actually the idea of #4574 was to remove #4544, But I didnt want to remove somebody elses work, so I improve it to a common use case that has the same problem.
A custom button could be created out of any library I just use material-ui as an example.

Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

On second read, this is correct, thanks for the explanation and sorry for the questions. Can you just cancel the white space changes in the rest of the file?

@WiXSL
Copy link
Contributor Author

WiXSL commented Apr 7, 2020

They are not white spaces, they are zero width unicode characters (in this case \u200B).
I asume someone add them accidentally.
But I'll revert them back if you want, no problems.

Please confirm about this.

@fzaninotto
Copy link
Member

You're right! How did you detect those? Some of us may have a bad setting in their IDE.

@fzaninotto fzaninotto merged commit 1dc5ca6 into marmelab:master Apr 7, 2020
@WiXSL WiXSL deleted the fix-docs-button-sanitation branch April 7, 2020 16:56
@WiXSL
Copy link
Contributor Author

WiXSL commented Apr 7, 2020

@fzaninotto

I'm using a plugin for IntelliJ-based IDEs.

https://plugins.jetbrains.com/plugin/7448-zero-width-characters-locator

@fzaninotto
Copy link
Member

Nice. I found this equivalent plugin for VSCode: https://marketplace.visualstudio.com/items?itemName=nhoizey.gremlins

@WiXSL
Copy link
Contributor Author

WiXSL commented Apr 7, 2020

Oh, I'll take note of that. Thanks.

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.

None yet

4 participants