Update module to use react-select #113
Conversation
|
Ok Travis pointed out that I lied, I've changed the format of the suggestion API on tagfield. I could change this to be backwards compatible? For now I just updated the tests, but I can revert that if we want backwards compatibility. |
|
master can have breaking changes, but whether it should for our internal integration effort is something we should consider. I'll leave you to weigh up the cost-benefit of keeping BC |
There was a problem hiding this comment.
Two minor comments, also can you remove the map files?
Regarding B/C, it would be ideal to release this as a minor update instead of a major release so if you can make it backwards compatible easily enough then I'd suggest we try and do that. If it's going to be too much effort though then we'll bite the bullet and bump it up.
client/src/components/TagField.js
Outdated
| return Promise.resolve({ options: [] }); | ||
| } | ||
|
|
||
| const fetchURL = url.parse(this.props.optionUrl, true); |
There was a problem hiding this comment.
Could you please dereference these props - lazyLoad, options and optionUrl? There's also some places in the render method that could use this too
client/src/components/TagField.scss
Outdated
| } | ||
|
|
||
| .Select-menu-outer { | ||
| border-top-color: #e6e6e6; |
There was a problem hiding this comment.
Is there a variable in admin somewhere we can use here?
package.json
Outdated
| "sass-loader": "^6.0.7" | ||
| "@storybook/addon-actions": "^3.2.19", | ||
| "@storybook/addon-options": "^3.2.19", | ||
| "@storybook/react": "^3.2.19", |
There was a problem hiding this comment.
Were you going to add a story book definition for it? That'd be cool, but not required
There was a problem hiding this comment.
I was playing around with that but I didn't get around to it. I can remove that.
|
I removed the BC changes so this can target the next minor now 🙂 |
|
If it's backwards compatible, can you make the silverstripe/framework dependency abit lower than 4.3 ? |
Resolves #110
This PR implements the change requested in the above issue. I added yarn and updated all the directory structure to match what new SS4 modules usually do. This is a backwards compatible enhancements so can me released in a minor - I have set the version in
package.jsonto 2.2.0 - for what that's worth.I noticed the implementation on the
disabledattribute on thereact-selectdependency isn't great - might want to look at raising an issue for that.