-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
feat: customization service append and customize functionality should run once #4238
feat: customization service append and customize functionality should run once #4238
Conversation
✅ Deploy Preview for ohif-dev canceled.
|
✅ Deploy Preview for ohif-platform-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
762ff5c
to
a7b3cb2
Compare
Passing run #4102 ↗︎
Details:
Review all test suite changes for PR #4238 ↗︎ |
…rvice-append-functionality
…rvice-append-functionality
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.
Fixing the use of the customization service to be correct.
@@ -3,7 +3,6 @@ const pkg = require('./package'); | |||
|
|||
module.exports = { | |||
...base, | |||
name: pkg.name, |
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.
name isn't a valid field, was causing spurious warnings during the unit test phase of this change.
@@ -160,5 +165,149 @@ describe('CustomizationService.ts', () => { | |||
expect(result.label).toBe(testItem.label); | |||
expect(result.value).toBe(props.testAttribute); | |||
}); | |||
|
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.
Add a whole bunch of unit tests to show how the services can be used/how to handle the append etc.
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.
Adds default customizations, which can be used to provide a basis for other changes, and added working merge and append functionality.
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.
Adding documentation on how to use the new functionality.
@@ -51,7 +51,7 @@ | |||
"react-dnd": "14.0.2", | |||
"react-dnd-html5-backend": "14.0.0", | |||
"react-dom": "^18.3.1", | |||
"react-draggable": "4.4.3", | |||
"react-draggable": "^4.4.6", |
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 fixes a persistent warning about react 18 incompatibilities.
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.
Thanks, looks great, i would simplify the append/merge in the docs later, i guess we can talk about the simple case instead of the advanced case
Context
Changes & Results
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment