-
Notifications
You must be signed in to change notification settings - Fork 13
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
Glitch when editing topic configs #143
Comments
My understanding is that a null value might be a valid value for a property. The table is data driven, but Kafka doesn't provide us with this information, so we can't really know without hardcoding the information in the UI. Clicking the X should do what you want tho, undo the change and go back to what you had earlier |
Yes this needs improvements. This error comes from Kafka itself and doesn't come annotated with the property name, so I can't show it underneath the right input as for other validation errors. Maybe a toast notification would be better for this kind of errors. Or maybe both the inline alert and a toast that scrolls the page to the inline. Or a toast with the error message and the inline one. |
It looks like |
@MikeEdgar I you're right that null is not supported. It should be safe to treat it as a deletion of the override for all existing topic configs. |
@tombentley are there any scenarios where a user should be allowed to set a config to an empty string? That would determine if we need something in the UI other than just the input field to determine whether the config should be blank/empty (but not null) or deleted and defaulted. |
@MikeEdgar that's exactly what I was trying to reason about. Right now I think the are no topic configs where the empty string is allowed but is not the default. Thus it's safe to treat empty as a delete. Nothing guarantees that in perpetuity, but I think that's a risk we should take for a simpler UX. |
Ok, I agree to keep it simple. From a back-end validation perspective, let's say that the server will reject any null or blank values and a delete operation will set the entire config to null (as the API expects already). This avoids representing a single action (deleting a config override) multiple ways in the API. The below JSON fragment shows examples of configs that would be rejected: "configs": {
"some.config": {
"value": " "
},
"some.other.config": {
"value": ""
},
"some.third.config": {
"value": null
}
} Removing a config (revert to default) is represented as: "configs": {
"some.config": null
} |
When editing topic configs (via the topic listing hamburger > "Edit properties"):
I expected clicking the tick when I haven't changed the value to behave the same as when I have changed the value.
The text was updated successfully, but these errors were encountered: