-
Notifications
You must be signed in to change notification settings - Fork 45
feat(registry/client): add possibility to block some settings #661
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
Conversation
bb4ed99 to
f4ce9e2
Compare
f4ce9e2 to
e7b6c0a
Compare
| error: error || settingError, | ||
| loaded: !isResourceLoading, | ||
| loading: isResourceLoading, | ||
| loaded: !isResourceLoading && 'true', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this condition looks strange. I assume that due to some weird types, but anyway if we can correct it would be good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an issue with React-Admin that complains in the console about wrong prop types passed to the component:
Warning: Received `true` for a non-boolean attribute `loaded`.
If you want to write it to the DOM, pass a string instead: loaded="true" or loaded={value.toString()}.
I assume it wants to be set as an attribute directly, not as a {...props} object.
After the update to the latest React-Admin, we'll probably remove this.
| Example: | ||
|
|
||
| ```bash | ||
| export ILC_REGISTRY_ADMIN_PROTECTED_SETTINGS="setting1,setting2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as far as I see it impacts frontend and webpack compilation, which means that if you set this setting on runtime - it won't have any effect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stas-nc the documentation says
Set this variable **before starting the application** to specify which settings should be blocked from editing in the admin UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was meant to be set before the webpack build.
I've added the possibility to change it in the container runtime as well, updated the documentation.
Coverage ReportIlc/serverCommit SHA:0cb94d6933e9d4f3c863beb66926247e99b64f5f Test coverage results 🧪File details
Ilc/clientCommit SHA:0cb94d6933e9d4f3c863beb66926247e99b64f5f Test coverage results 🧪File details
RegistryCommit SHA:0cb94d6933e9d4f3c863beb66926247e99b64f5f Test coverage results 🧪File details
|
This PR introduces the ability to block/protect certain settings from being modified in the registry client. Fixed some ui errors as well.