-
Notifications
You must be signed in to change notification settings - Fork 52
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
Max Width & Callback Improvements #21
Conversation
* Add basic support for `maxWidth` for Panels. This can be specified just like "minWidth", and defaults to 0 * Add a second argument to the `onUpdate` callback, allowing more seamless integration with external state libraries Additionally, code has been refactored to break apart Divider, Panel, and Panel Group into standalone components. Remaining TODO: * Add tests covering the `maxWidth` and `isComplete` behaviors.
Codecov Report
@@ Coverage Diff @@
## master #21 +/- ##
=========================================
- Coverage 9.89% 9.37% -0.52%
=========================================
Files 1 3 +2
Lines 182 192 +10
=========================================
Hits 18 18
- Misses 164 174 +10
Continue to review full report at Codecov.
|
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.
Love this PR. Thanks for the contribution. Couple of questions for you.
onResizeComplete = (i) => { | ||
var tempPanels = this.state.panels.slice(); | ||
|
||
tempPanels[i].updated = true; |
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.
What is this tempPanels[i].updated property used for?
e.stopPropagation() | ||
e.preventDefault() | ||
|
||
return this.onResizeComplete(this.props.panelID) |
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.
Aside from this new function, were there any other changes two Divider or Panel - I can't easily see the changes since you separated them into their own files
@DanFessler I'll revisit this over the weekend ... been a busy few weeks since pushing this 🤹♂️ |
@pdfowler sorry for the ping, but would you be able to get back to this PR any time soon? Being able to specify max width is a highly desirable feature for me and I'd love to see it land sooner rather than later, so I'd be willing to lend a hand or take over this PR if that OK with you (and maybe scope it down to just that feature, or break it into several PRs, if it would be easier to review for @DanFessler) |
So sorry - we’ve been on a pretty aggressive push for the last couple of months and I’m not sure when I can get back to it ... 😩. Sorry - and I’ll try to get in and address your questions tonight.
…Sent from my iPhone
On Apr 21, 2018, at 10:25, Serg Nesterov ***@***.***> wrote:
@pdfowler sorry for the ping, but would you be able to get back to this PR any time soon?
Being able to specify max width is a highly desirable feature for me and I'd love to see it land sooner rather than later, so I'd be willing to lend a hand or take over this PR if that OK with you (and maybe scope it down to just that feature, or break it into several PRs, if it would be easier to review for @DanFessler)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
max width merged with #27. If you'd like to submit a new PR for the onUpdate callback, feel free. I don't really understand its utility. |
This PR implements the following features:
maxWidth
for Panels. This can be specified just like "minWidth", and defaults to 0 (ie: disabled)onUpdate
callback, allowing more seamless integration with external state librariesAdditionally, code has been refactored to break apart Divider, Panel, and Panel Group into standalone components.
Remaining TODO:
maxWidth
andisComplete
behaviors.