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
parameter & operation extensions display #3868
Conversation
@thompsongl thanks for the PR. I'll leave it to @shockey to comment about the code itself. Running this with a definition that has an extension in a parameter - am I supposed to see anything? Could be completely blind... |
@webron Not blind. I had an uncommitted change hanging out 😳 |
I think for an initial version, the styling is ok. Not a lot of magic that can be done with it. However, I think rendering the extensions should be optional and by default should be false. |
I can make that happen. Optional by way of configuration parameters or another method? |
Yeah, a configuration parameter. And yeah, would be great if that would apply to model extensions as well. Thanks for the help. |
@thompsongl, I just merged #3875 😄 |
Updated: Uses configuration param to display extensions |
@shockey, anything else you'd like to see as part of this changeset? |
@thompsongl, all good here - hoping to get this reviewed and merged Monday 😄 |
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.
LGTM. Thanks for adding the documentation, and updating the relevant Enzyme test.
For future PRs, more comprehensive tests would be nice 😄
@@ -7,6 +7,7 @@ export default class ArrayModel extends Component { | |||
static propTypes = { | |||
schema: PropTypes.object.isRequired, | |||
getComponent: PropTypes.func.isRequired, | |||
getConfigs: PropTypes.func.isRequired, |
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.
I'm not a huge fan of the getConfigs
handaround - would prefer to see a container component mapping from the configs to props for children.
Fixes #3052 by adding components to display Parameter and Operation extensions (
x-
values)Presentational components for
x-
value displays are small and explicit intentionally. These changes allow for baseline rendering while giving easy access to override/wrap each extension component granularly. For instance, we plan on wrapping operation extensions to display extra information about our security-related values.Considerations: