-
Notifications
You must be signed in to change notification settings - Fork 24
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
chore: preparation to NPM publish #1511
base: develop
Are you sure you want to change the base?
Conversation
Signed-off-by: Viktor Tsvetkov <[email protected]>
Signed-off-by: Viktor Tsvetkov <[email protected]>
Signed-off-by: Viktor Tsvetkov <[email protected]>
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.
just some minor questions, but looks good, interfaces seems to be reflected correctly in classes, order of all props in constructors is the same
"license": "Apache-2.0", | ||
"private": true, | ||
"author": "Splunk Inc.", |
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.
Should we add this one?
"name": "@splunk/ucc_ui_lib", | ||
"version": "0.0.1", | ||
"name": "@splunk/add-on-ucc-framework", | ||
"description": "Splunk Add-on SDK formerly UCC is a build and code generation framework", |
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.
formerly?
) => CustomTabInstance<T>; | ||
|
||
export abstract class CustomTabBase { | ||
protected tab: object; |
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.
maybe lets add a type for this one?
const customControl = new Control( | ||
globalConfig, | ||
this.el, | ||
this.props.data, | ||
this.setValue, | ||
this.props.utilCustomFunctions | ||
); | ||
customControl?.render(); | ||
customControl.render(); |
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.
to not change behaviour, should we validate if that one exists?
Issue number: ADDON-76791
PR Type
What kind of change does this PR introduce?
Summary
Changes
User experience
There is no impact on the app logic. Except there was a missing groupName param for CustomHookConstructor: ui/src/components/BaseFormView/BaseFormView.tsx:1108
Checklist
If an item doesn't apply to your changes, leave it unchecked.