Fix DropDownBox height auto resizing (T685238) #6317
Conversation
js/ui/popup.js
Outdated
| }, | ||
|
|
||
| _renderAutoResizableGeometry: function() { | ||
| this._toggleContentAutoResizableClass(); |
There was a problem hiding this comment.
Why do you need this method? Can you insert this._toggleContentAutoResizableClass in the top of the renderGeometry method?
js/ui/popup.js
Outdated
| @@ -404,6 +407,7 @@ var Popup = Overlay.inherit({ | |||
| var isFullscreen = this.option("fullScreen"); | |||
|
|
|||
| this._toggleFullScreenClass(isFullscreen); | |||
| this._toggleContentAutoResizableClass(); | |||
There was a problem hiding this comment.
Render calls the _renderGeometry method. This call can be in the top of the renderGeometry
| @@ -413,6 +416,18 @@ var Popup = Overlay.inherit({ | |||
| .toggleClass(POPUP_NORMAL_CLASS, !value); | |||
| }, | |||
|
|
|||
| _shouldUseContentAutoResizableClass() { | |||
| if(this.option("height") !== "auto" || this.option("shading") || this.option("fullScreen") || this.option("showTitle") || this._getToolbarItems("bottom").length) { | |||
There was a problem hiding this comment.
Why the popup should not be auto-resized if shading/showTitle/bottom toolbar is enabled? I think it is good when the dxPopup will be resized in all cases when the user sets the auto height
| return false; | ||
| } | ||
|
|
||
| return this.option("useAutoHeightWithLimits"); |
There was a problem hiding this comment.
We should write a test for each case that could be broken if we will remove this option. Maybe this will not be a breaking change and all cases are expected.
| flex-direction: row; | ||
| } | ||
|
|
||
| .dx-overlay-content { |
There was a problem hiding this comment.
Does the fix work without .dx-overlay-content css?
The possibility to move this css to the Overlay level should also be checked
There was a problem hiding this comment.
Use the ".flex-container" mixin instead of this
There was a problem hiding this comment.
display: flex;
flex-direction: row;
Everything this has inside the flex-container mixin which supports flex styles with other prefixes (browsers compatibility)
There was a problem hiding this comment.
@jsdmitry We already have autoprefixer for browsers compatibility
| @@ -413,6 +416,18 @@ var Popup = Overlay.inherit({ | |||
| .toggleClass(POPUP_NORMAL_CLASS, !value); | |||
| }, | |||
|
|
|||
| _shouldUseContentAutoResizableClass() { | |||
| if(this.option("height") !== "auto" || this.option("shading") || this.option("fullScreen") || this.option("showTitle") || this._getToolbarItems("bottom").length) { | |||
There was a problem hiding this comment.
| if(this.option("height") !== "auto" || this.option("shading") || this.option("fullScreen") || this.option("showTitle") || this._getToolbarItems("bottom").length) { | |
| return this.option("height") === "auto" && !this.option("fullScreen"); |
|
|
||
| .dx-overlay-content { | ||
| display: flex; | ||
| position: relative; |
There was a problem hiding this comment.
| position: relative; | |
| flex-direction: column; |
| flex: 1 1 auto; | ||
| min-height: 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
| } | |
| .dx-popup-title, .dx-popup-bottom { | |
| flex-shrink: 0; | |
| } | |
| } |
|
The solution with flex styles has a lot of problems (especially in IE11). I try to use another way to solve the issue |
No description provided.