Primeng: Dialog positioning improvement

[maplion]

I'm submitting a ... (check one with "x")

[x] bug report => Search github for a similar issue or PR before submitting
OR
[x] feature request => Please check if request is not on the roadmap already https://github.com/primefaces/primeng/wiki/Roadmap
[ ] support request => Please do not submit support request here, instead see http://forum.primefaces.org/viewforum.php?f=35

Plunkr Case (Bug Reports)
Please fork the plunkr below and create a case demonstrating your bug report. Issues without a plunkr have much less possibility to be reviewed.

http://plnkr.co/edit/dWh4IJoFdiNDdQVMJwSE

Note: I will potentially add a demo later; do not have time at the moment.

Current behavior

I have a responsive dialog that loads content dynamically and the same component is reused for many different viewings (accessed from buttons on a data table). It loads the first time, centered from its top left corner to bottom right, receives its data asynchronously, and then is off center. It remembers where it was and the data it had prior, so if I view another set of data, it is properly centered horizontally, but depending on what data it got back previously, it changes its position vertically. The result is a dialog that moves all over the place depending on what content was loaded previously.

Is there a way to center the dialog after the content loads?

Current workaround is to set the dialog with a static width and height, making it not responsive.

Expected behavior

Either center after content is loaded or be able to control / set the dialog to center after it has loaded.

Minimal reproduction of the problem with instructions

What is the motivation / use case for changing the behavior?

Viewing data based on table rows (pop-up dialogs triggered from a button within the row of the data grid).

Please tell us about your environment:

Windows 10, Visual Studio Code, NPM

• Angular version: 4.1.0
  

• PrimeNG version: 4.0.0-rc3
  

• Browser: [Chrome XX | Firefox XX | IE XX ]
  

• Language: [all | TypeScript 2.3.2 | ES6/7 | ES5]

• Node (for AoT issues): node --version = 6.10.0

[dhniels]

I'm having this same problem as well.

[ghost]

Same problem here.
I add dynamic content, and the dialog is not recentered vertically. I can't see the bottom of my dialog.

[huineng]

i solved this by adding a little time out , this gave some time to do the dimension recalculation

 setTimeout(() => {
                this.dialog.showDialog = true;
            }, 0);

[ghost]

I can't do that in my case, because it's an action in the dialog.

[SIlver--]

Having the same issue. When I change the width and height attributes with the dialog open, it does not recenter.

[saximoer3]

i solved this by using 2 ways, but i think there should be better ways.

1. wait until the data of content loaded, then
   setTimeout(() =>{
   this.display = true;
   },10);

1. i give the dialog a static position after it show.

HTML: (i put i on the top)

TS:
import {ElementRef} from '@angular/core';

           constructor(private el: ElementRef,) { }

           onShow() {
                    setTimeout(() =>{
                            this.el.nativeElement.children[1].childNodes[0].style.left = '200px';
                            this.el.nativeElement.children[1].childNodes[0].style.top = '20px';
                     },10);
            }

[dhniels]

id like to see a more ideal, built-in solution to this from the PrimeNG team. Loading dynamic content in a dialog seems like a pretty common use case.

[dhniels]

@saximoer3 , i like your 3rd solution but couldnt get it to work. my dialog is still cutting off at the bottom of the window. could you provide a plunker example?

[cagataycivici]

I'll add alwaysCenter property to center it ngAfterViewChecked. So when something changes, after dom is ready, we'll recenter. Should solve it, hopefully without side effects, default value will be false though.

[saximoer3]

@dhniels
i couldnt get it to work too. i use the 2nd way now. When loading dynamic content, it should wait until well done. so i remove it.

[dhniels]

@cagataycivici thank you so much! i know many people will be happy about this new feature!

[negberts]

I am using setTimeout(this.dialog.center.bind(this.dialog), 0); in the complete part of the HTTP observable. It centers nicely then.

[cagataycivici]

Ok now Dialog centers itself after its content is ready, I've also realized that this fixes some positioning issues in the demos like DataGrid demo where content is dynamic.

[dhniels]

Awesome! Do we need to update the PrimeNG version or set an attribute or does it just work by default now?

[cagataycivici]

Should work just by default after 4.1.RC3 (tomorrow)

[bakasmarius]

Still does not seem to work in 4.1.2.
Anyone else still having the same problems, or is it just me?
P.S.: timeout helps though.

[ivanhosgood]

I can confirm that this still does not work on 4.1.2.
Timeout is useful for a static dialog but in my scenario the dimensions change depending on the quantity of data which changes over time. Currently hiding and re-displaying the dialog after timeout helps but this hack results in flickering

[jacqueslareau]

I think this issue needs to be reopened. Dimensions almost always changes with async data. @cagataycivici

[dhniels]

@jacqueslareau agreed. my dialogs all have dynamic content. this new "responsive" and "alwaysCenter" setting just simply doesnt work. confirmed not working on 4.2.2

[nikolatzotchev]

+1

[lasimone]

+1

[vertexportus]

+1

[bakasmarius]

@cagataycivici may we know your thoughts about this?

[rich4756]

Can we please reopen this?

[dhniels]

@rich4756 9 months since this issue was opened, i think its safe to say @cagataycivici and the PrimeNG team are ignoring this. This has been my experience on a number of significant bugs with the product.

[dhniels]

I was working on a new dialog today where it loads with a table hidden (via *ngIf) and once it inserts the table into the dom, it pushes the dialog down and right, and when I move the dialog around the right edge stays glued to the ridge edge of the viewport.

cant believe i've run into this issue again. It's truly shocking this is still an issue 9 months later and that this was even closed in the first place. PrimeNG 5.2.0

[noamichael]

I agree with @dhniels . Can we get an ETA?

[cagataycivici]

Reopened due to community request, will be reviewed for 5.2.1. Regarding issue tracker in general, we are doing everything we can, we've reviewed/resolved 3788 issues so far and there are 533 open where most of them are feature requests. PRs will definitely help in general, let's improve it together.

[thenninger]

Was this suppose to be added to 5.2.1? It's still closed - sorry just confused. I'm experiencing centering issues with the confirm dialog as well.

[cagataycivici]

I forgot to reopen after my previous comment.

[cagataycivici]

My test is this;

<p-dialog header="Godfather I" [(visible)]="display" modal="modal" [responsive]="true" [width]="300" [minWidth]="200">
           <p>Unrelated content.</p>
           <p *ngIf="displayDynamic" style="height:100px">
               Dynamic Content
           </p>
           <ul>
               <li *ngFor="let item of items">{{item}}</li>
           </ul>
           <p-footer>
                <button type="button" pButton icon="fa-search" (click)="displayDynamic = !displayDynamic" label="Toggle"></button>
                <button type="button" pButton icon="fa-check" (click)="add()" label="Add"></button>
           </p-footer>
    </p-dialog>

So one button toggles the visibility of a p element with 100px height and other ones adds items to the list displayed by the list. These two buttons are used to change the content of the dialog dynamically.

I've seen the issue where the dialog position stays same in this case and can think of a way to take this.

If at ngAfterViewChecked callback of dialog.ts itself, we call positionOverlay() function, then after the change detection runs and view is updated, dialog realigns itself. My concern here is that this function is called many many times by Angular so we need to do this if dialog is visible. If it is hidden then it is positioned already after it gets shown so both cases are covered.

I've committed the change, please test it with 5.2.1 due March 7th and drop a comment here about the results. The change fixes the issues of my test case and hopefully yours.

[dhniels]

Just installed 5.2.1 and from a quick test it appears to be working! hooray!

[negberts]

I'm getting seasick now... See attachment.. (https://github.com/primefaces/primeng/files/1792475/ice_video_20180308-101247.zip)

Any way the continuing dialog positioning can be disabled?

[dhniels]

@negberts mine is doing this as well in one place when i open a dropdown. it starts twitching up and down like that.

here is @negberts video but in gif format for convenience of viewing it directly on here:

[dhniels]

Heres another example: (dont know if it matters but the dropdown uses append to body)

the whole dialog moves around and makes me miss clicks, or have to click twice to open it.

to be clear - i want the centering feature with dynamic content, but maybe theres a way we can fix it from jumping pixels on stuff like this...

[cagataycivici]

Is this on 5.2.2? It seems we should avoid reposition when content does not change. Will check it now.

[dhniels]

@cagataycivici yes, 5.2.2

[cagataycivici]

New idea was to check the height at on show and at afterViewChecked, check to see if height is different which means the content has changed dynamically, if so then reposition;

        else if(this.visible) {
            this.zone.runOutsideAngular(() => {
                setTimeout(() => {
                    let height = this.domHandler.getOuterHeight(this.containerViewChild.nativeElement);

                    if(height !== this.currentHeight) {
                        this.currentHeight = height;
                        this.positionOverlay();
                    }
                }, 100);
            });
        }

It fails without setTimeout as Angular's ngAfterViewChecked is somewhat broken, it gets invoked before the dom update is finished so I can't check the new height without it, ngZone is there to avoid recursive calls, since setTimeout will call ngAfterViewChecked again.

This is the only potential solution to reposition.

Another workaround is fixing the content section;

<p-dialog [contentStyle]="{'max-height': '400px'}"

So that dynamically changed content does not change the coordinates of the dialog, a scrollbar will display for longer content.

Wdyt?

Maybe we can do the automatic positioning a property like autoAlign="true|false"?

I'm leaning towards max-height on contentStyle, it seems to be a valid workaround.

[dhniels]

@cagataycivici I dont mind the idea of fixing the height of the content section, _however_ it doesnt seem to work with vh or percent units. i use a max-height currently in vh in all our modals to allow overflow but allowing it to grow responsively with the screen size.

are there any downsides to the first solution that you can see in your testing? it appears that the dialog currently recenters when i click. im wondering why the dropdown is causing this however, if it is appended to the body? shouldnt the dialog only consider the content inside of .ui-dialog-content? heres another screen capture better illustrating this (click in, click out - reposition. click out again - reposition.):

i wonder if adding a property is necessary with dynamic content. would you ever want the dialog to have the ability to grow beyond the viewport and NOT center? that seems odd to desire that behavior. seems to me that it should be the default behavior, not attached to a property...

[cagataycivici]

Ok, I've changed the auto align behavior to check the height instead. It is on by default but can be turned off with [autoAlign]="false" just in case. Also added a doc and released 5.2.3, try with it please.

[dhniels]

@cagataycivici Good idea. i think that will work. Ill make sure to test once 5.2.3 is released

[cagataycivici]

5.2.3 is released now.

[dhniels]

youre the man @cagataycivici ! i think that worked. I will test, specifically test vh units - a case where if the top value is being calculated on subpixels, ex. between 38.5px and 39px. Im worried it might cause that issue where it "twitches" back and forth between the whole and half pixel. im wondering if thats whats causing @negberts issue....

hopefully the angular team will fix the issues with ngAfterViewChecked that require you to use the set timeout. then a proper solution can be implemented :)

in either case, this is the best dialog i've seen yet. i think youve finally resolved some longstanding issues with a solution that (hopefully) remains bug free! Congrats!

[Treynis]

With [autoAlign] = "false" problem with shaking inside of a dialog is gone. Thanks!

[cagataycivici]

Thank you, since it is stable now I can add maximize minimize like PrimeFaces has😉

https://www.primefaces.org/showcase/ui/overlay/dialog/minMax.xhtml

[negberts]

Works like a charm!

[jonbarthol]

Thanks. It's nice to have a workaround. However, it's unfortunate that you have to set [autoAlign]="false" as @Treynis stated above. Doing this does not allow the dialog to recenter itself after the height is dynamically changed.

There is an "off by one pixel" problem with either Angular or the browser that causes the height of the containerViewChild to vary by one pixel every time ngAfterViewChecked is called. This only seems to be a problem when the viewport height is an odd number of pixels. @cagataycivici what do you think of changing the "if" condition in ngAfterViewChecked from:

if(height !== this.currentHeight)

to:

if(height < this.currentHeight - 1 || height > this.currentHeight + 1)

This would resolve the "off by one pixel" problem. Thanks.