Fix clear button not including select inputs #283
Conversation
| onClearRequestData(e) { | ||
| const requestPanelEl = e.target.closest('.request-panel'); | ||
| const requestPanelInputEls = [...requestPanelEl.querySelectorAll('input, tag-input, textarea:not(.is-hidden)')]; | ||
| const requestPanelInputEls = [...requestPanelEl.querySelectorAll('input, select, tag-input, textarea:not(.is-hidden)')]; |
There was a problem hiding this comment.
I actually think select needs special handling, for instance what if select doesn't have a ' ' option, for optional enums for instance, the values include null not ' '. And for required enums ' ' isn't an option. Realistically we would need to select the default value in cases where the parameter is required and the nullish version that matches the property for types were it is required. I don't know if the properties already exist to figure that out from the html attributes....
There was a problem hiding this comment.
I'm not entirely sure how required parameters should behave when hide-defaults is enabled. What should be the initial value and what should "reset" reset them to? My new revision ignores the hide-defaults for required parameters and uses the defaultValue if available.
There was a problem hiding this comment.
Two things also come to mind here:
- How is "initial" different from "default" here. I think default is what these should be reset to.
- We can never use
||because that breaks nullable and falsy default values.
I think this is going to take some more serious attention.
Rather than this latest commit, I would suggest going back to what you had before, but adding in special case handling for picking an allowed value (or null or false or '') for enums for the reset option. It might be just something like defaultValue !== undefined ? defaultValue : allowed values[0] but I haven't thought through that all the way.
|
I'm applying this changes in #286, closing this PR. |
2 participants
No description provided.