Dialog resizing problem update & solution

[mmikeyy]

I posted about this problem a while ago and got zero replies.

I understand why now. Not that I was wrong. But I understand now why not many people, even possibly no one, saw the problem.

Unless I'm totally wrong of course, there is a big flaw in the dialog resizing method. But for it to become apparent, a few conditions must be met. Nothing extraordinary.

The biggest problem stems from the logic used. The method is tracking the mouse one pixel at a time, and depending on the computer's speed and the mouse's speed, the size adjustments are always calculated between two consecutive positions. This means that size adjustments of a single pixel are frequent. And this is a problem.

It is a problem because... the element's initial height is derived from its offsetHeight property. Then it's adjusted by subtracting margin and border widths. BUT, and here lies the problem, it is assumed that these are integral values. The method making the adjustment (domhandler.js#getHeight) neglects to take into account that widths may be set in other units, which may result in fractional pixel dimensions.

Imagine a padding value in em's resulting in 7.7px. When the padding width is treated as an integer, we obtain 14px total for top and bottom, with an error of 1.4px. And the adjustment we're making is just 1px. The error in measuring the current height is greater than the adjustment we're making. And when you try to shrink the window, it can grow uncontrollably larger instead.

The solution to this problem is to correct the DomHandler.getHeight method as follows:

Code: Select all

    DomHandler.prototype.getHeight = function (el) {
        var height = el.offsetHeight;
        var style = getComputedStyle(el);
        // height -= parseInt(style.paddingTop) + parseInt(style.paddingBottom) + parseInt(style.borderTopWidth) + parseInt(style.borderBottomWidth); // remove this line
        height -= parseFloat(style.paddingTop) + parseFloat(style.paddingBottom) + parseFloat(style.borderTopWidth) + parseFloat(style.borderBottomWidth); // use this instead
        return height;
    };

But this is not the only problem.

I'm using a library that sets the CSS property box-sizing to 'border-box'. When the height is set, it is assumed by the Dialog#onResize method that setting the height sets a dimension excluding padding and border. But if box-sizing is set to 'border-box', adjusting for border and padding is not necessary.

With another change to DomHandler#getHeight, the problem goes away:

Code: Select all

    DomHandler.prototype.getHeight = function (el) {
        var height = el.offsetHeight;
        var style = getComputedStyle(el);
        if (style.boxSizing == 'content-box') { // make height adjustment conditional
          height -= parseFloat(style.paddingTop) + parseFloat(style.paddingBottom) + parseFloat(style.borderTopWidth) + parseFloat(style.borderBottomWidth);
        }
        return height;
    };

Another solution to this css problem would be for users to add a CSS rule such as (sass):

Code: Select all

div.ui-widget div.ui-widget-content
  box-sizing: content-box!important

Perhaps it could be added to primeng.css.

In any event, the parseInt problem needs to be fixed. Or the logic should be changed to something like the one I proposed in a previous post on this topic. I proposed to recalculate the size relative to the initial size when resizing started, not using a delta between two consecutive positions. Fractional pixel errors do not accumulate and are negligible with this logic.

[mmikeyy]

Me again. Sorry for being so insistant.

Just want to point out that if the reader finds my description of the problem too convoluted or unconvincing, it's easy to observe it on Primeng's own demonstration page.

Proceed as follows.

1) go to the dialog section;
2) open the dialog (click the 'show' button')
3) open your Developer Tools window
4) locate the dialog's div.ui-dialog-content element.
5) set its padding to 1.45em.
6) go back to the page and have fun trying to resize the dialog.

The padding in the style below was added by myself as described above.
The height style value was set by the dialog component as the dialog was resized. Notice the unreasonable value. That's what happens when the mouse is moved only a short distance. The dialog can grow or shrink uncontrollably, depending on the padding value.

Code: Select all

<div class="ui-dialog-content ui-widget-content" style="padding: 1.45em; height: 1283px;">
                
        <p>The story begins as Don Vito Corleone, the head of a New York Mafia family, oversees his daughter's wedding. 
           His beloved son Michael has just come home from the war, but does not intend to become part of his father's business. 
           Through Michael's life the nature of the family business becomes clear. The business of the family is just like the head of the family, 
           kind and benevolent to those who give respect,
           but given to ruthless violence whenever anything stands against the good of the family.</p>
        
    
            </div>

One might be tempted to reply that I just have to avoid messing with the html generated by the component.

But the padding value is actually based on CSS, and users are free to use any units they want. I stumbled on this problem accidentally because my CSS sets padding in em units for dialog contents. This may result in fractional numeric values when the converting to px, which the component can't handle.

[mmikeyy]

All of the above posts to which I'm replying are still valid in RC5.

Just upgraded from RC4, and the first dialog I resized grew uncontrollably. It's easy to replicate. The reason is known. It's easy to fix. Why not do it?

At the present time, resizing works or not depending on css values that the user is free to set to any value he likes. If padding is set in 'em's and the font size is such that the resulting padding values in px are not integral, the height calculation for the contents can be off by several pixels. When the resizing algorithm makes adjustments 1px at a time to a current height that is repeatedly measured (after each adjustment) with an error of several px, is it not easy to understand how things can go wrong?

If you don't see the problem, just keep changing padding (in fractional em, e.g. 1.33 em) and/or font-size. It won't be long before your dialogs are out of control.