feat: horizontal mode #124
Conversation
|
|
Thanks for the PR! I'm actually working on a major refactor in #122 that will make this difficult to merge. Once that is completed, this PR will need to be rebased against it. |
Great news @roginfarrer ! Do not worry i just read the code and it looks VERY CLEAN 😃 , i will wait for your release then i'll re-implement the horizontal mode. |
❌ Deploy Preview for react-collapsed failed.
|
|
Hey @roginfarrer |
packages/core/src/Collapse.ts
Outdated
| this.setStyles({ | ||
| ...this.getCollapsedStyles(), | ||
| // When expand = false and collapsing width we preserve initial height | ||
| ...(this.options.isHorizontal ? {height: `${this.initialHeight}px`} : {}), |
There was a problem hiding this comment.
I think this might be an issue with an element that's made the full height of the window. An example would be a drawer or a sidebar. Upon resizing, hardcoding the height would cause the sidebar to not stay adhered to the window edge.
We might need to use a resizeobserver in order to update the height
There was a problem hiding this comment.
Or maybe it should be an option to persist the height when the width is animated and collapsed? Not sure which is better
There was a problem hiding this comment.
There is two places where we persist the height:
- In the init method when the default expand is equal to false.
- In the close method when the width is animated and collapsed.
If we are going to use resizeobserver (observe height) on the element it will interfere with collapase when it is changing the height for animated width.
I think the second option you mentioned is already handled in close method and for me i go for this option, but if you have any suggestions to improve it let me know please ;)
There was a problem hiding this comment.
I pulled up the storybook, and it looks like the fixed height is always applied (it would need to be removed in the transitionend handler. I made an example in the example app that shows this quick and dirty sidebar demo.
There was a problem hiding this comment.
Good point, i think we should reset height on transitionEnd then I'll check other scenarios
There was a problem hiding this comment.
Actually responsive and fixed value of height will work perfectly without saving initial height.
The only issue that we will have is with the auto height, it is changing while animating width, so i think in this case it is up to the user to set the option collapseStyles what do you think ? I tried a solution by adding a setTimeout to detect if height is changed at middle of transition and it works ! but if the default expand is false at the initialization there is no chance to have this information ^^
Co-authored-by: Rogin Farrer <[email protected]>
Co-authored-by: Rogin Farrer <[email protected]>
Co-authored-by: Rogin Farrer <[email protected]>
This feature add an horizontal mode (width) to the collapser
enhancement request from #81
Without collapasedWidth option
With collapasedWidth option