-
Notifications
You must be signed in to change notification settings - Fork 1.2k
lib: Introduce the Dialog Implementation Convenience Kit #22700
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
base: main
Are you sure you want to change the base?
Conversation
bdda053 to
2dd5f8c
Compare
b174978 to
39357ad
Compare
454c423 to
18df4c6
Compare
0d4455c to
bb59c19
Compare
| - value.remove(index) | ||
|
|
||
| If the current value is an array, remove the element at "index". | ||
| It is important to use this function instead of just "value.set()" |
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.
Is it a "bug" or performance loss to use set over all of the add, remove, etc?
I feel like using set and pass it a modified version of array is more of a "javascript" approach similarly how you use setState and give it a new array instead of in-place editing which is not right.
Or maybe not a JS approach but immediately obvious way how to handle this :)
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.
ah, I should have read the block bellow 👍🏻
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.
Is it a "bug" or performance loss to use set over all of the add, remove, etc?
Yeah, it's not a bug, but very good practice to avoid spurious re-validation.
| dlg.value("list").forEach(v => { | ||
| v.validate(vv => { | ||
| if (vv == ".") | ||
| return "No dots"; |
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 added line is not executed by any test. Details
| await cockpit.spawn(["ls", "--no-such-option"], { err: "message" }); | ||
| } else if (values.error == "random") { |
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.
These 2 added lines are not executed by any test. Details
| /> | ||
| { | ||
| // Calling "map" on a non-array should just do nothing. | ||
| dlg.value("text").map((v, i) => <span key={i}>{v.get()}</span>) |
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 added line is not executed by any test. Details
This is already used in