Skip to content
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

bug(Datepicker): Calendar's position is being switched to upside even when there's no space to fit it #6965

Closed
julianobrasil opened this issue Sep 10, 2017 · 13 comments · Fixed by #11607
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@julianobrasil
Copy link
Contributor

julianobrasil commented Sep 10, 2017

Bug, feature request, or proposal:

Bug.

What is the expected behavior?

(Not sure) If there's no space in the screen nor bellow neither above to show the entire calendar, the bellow position should be the chosen one, so the user could scroll down if necessary (the scrollbar should be shown in this case).

Even better, when there's not enough screen space nor above neither bellow (but enough space in visible browser window viewport) the calendar could be shifted from its base position (defined by the the current rules) trying to fit itself in the available space. But the offset would be limited by the associated input (the input underline should fall in the space between the top and bottom edges of the calendar). Finally, when the total visible screen space wasn't enough to fit the calendar, it would end up in the bellow position, and the scrollbar would be visible.

What is the current behavior?

Notice that the top of the calendar was cropped in the images bellow and there's no scrollbar so one can see the hidden part of it (in fact, there's not viewport above to be scrolled to reveal the hidden part). And it looks like there is space to show it (first image) if it's given an offset.

IMAGE 1:

image

IMAGE 2:

image

IMAGE 3:

image

What are the steps to reproduce?

  1. Open the plunk: https://plnkr.co/edit/QeV9drIbHGEPS4ECFSZ5?p=preview
  2. Shrink the browser window so the calendar doesn't fit in it

What is the use-case or motivation for changing an existing behavior?

Better user experience when working in not-full-screen/small windows

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

@angular/cli: 1.4.0
node: 6.10.3
os: win32 x64
@angular/animations: 4.3.6
@angular/cdk: 2.0.0-beta.10
@angular/common: 4.3.6
@angular/compiler: 4.3.6
@angular/core: 4.3.6
@angular/flex-layout: 2.0.0-beta.9-5f198a3
@angular/forms: 4.3.6
@angular/http: 4.3.6
@angular/material: 2.0.0-beta.10
@angular/platform-browser: 4.3.6
@angular/platform-browser-dynamic: 4.3.6
@angular/platform-server: 4.3.6
@angular/router: 4.3.6
@angular/cli: 1.4.0
@angular/compiler-cli: 4.3.6
@angular/language-service: 4.3.6
typescript: 2.3.4

@julianobrasil julianobrasil changed the title bug(Datepicker): Calendar's position is being swithced to upside even there's no space to show it bug(Datepicker): Calendar's position is being switched to upside even there's no space to show it Sep 10, 2017
@julianobrasil julianobrasil changed the title bug(Datepicker): Calendar's position is being switched to upside even there's no space to show it bug(Datepicker): Calendar's position is being switched to upside even there's no space to fit it Sep 10, 2017
@julianobrasil julianobrasil changed the title bug(Datepicker): Calendar's position is being switched to upside even there's no space to fit it bug(Datepicker): Calendar's position is being switched to upside even when there's no space to fit it Sep 11, 2017
@josephperrott josephperrott added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent responsive labels Sep 27, 2017
@julianobrasil
Copy link
Contributor Author

Any news about this? One user complained today (I told him to press F11 to change the browser to full screen mode).

image

Maybe I use the touch UI mode for now or other users can be confused.

cc @mmalerba

@mmalerba
Copy link
Contributor

I believe it tries to position the calendar below and if there's not enough room it tries to position it above. If there's still not enough room it chooses whichever had more space available (in this case above). But I agree that it doesn't work so well here... I believe @jelbourn is working on a new positioning strategy that pushes the content back on screen. Is that ready yet Jeremy?

@jelbourn
Copy link
Member

It's still in-progress, should be soon-ish

See #6534

@julianobrasil
Copy link
Contributor Author

julianobrasil commented May 30, 2018

Hi, @mmalerba and @jelbourn, I realized that #9153 was merged in March but, this undesired behavior is still there. The datepicker calendar, IMO, should be shifted down when there's no room above to show the header.

It's better to hide part of the body at the bottom than hide the header (as, theoretically, you could scroll down to get to the bottom). And here I see an additional problem: even if you have the possibility to scroll up the page while the calendar is opened to try to make the calendar header visible, the scrolling is locked. Wouldn't it be better to release the scrolling while the calendar is opened?

In fact, I should be able to tell the overlay to consider a page offset from top/bottom. Look at the image below: shouldn't I be able to tell the position strategy to not cover the top navbar, by considering the page available space to start below the navbar (somehow telling the position strategy to offset from top/bottom by an amount of pixels)?

image

@blackblood19
Copy link

can you please let me know the progress on this issue? Looks like the pull request is in progress for the last 50 days..

@julianobrasil
Copy link
Contributor Author

julianobrasil commented Jul 21, 2018

@blackblood19, as a workaround you can switch your datepickers to touchUi when it's necessary. I'm using this directive to accomplish this task automatically (if you change the viewport size while the calendar is opened it becomes a little messy, and it's back to normal if you close and open the calendar again => currently I can live with that since most of the time the viewport size is defined only when the browser is opened for the first time):

<mat-datepicker appAutoModeSwitch></mat-datepicker>
@Directive({
  selector: '[appAutoModeSwitch]',
})
export class AppDatepickerSwitchDirective implements OnInit, OnDestroy {
  _isShortScreen = false;

  private _monitorScreenHeight$ = fromEvent(window, 'resize');
  private _destroy$ = new Subject<void>();

  constructor(
    private _zone: NgZone,
    @Self()
    @Host()
    private datepicker: MatDatepicker<any>,
  ) {
    this._zone.runOutsideAngular(() => {
      this._monitorScreenHeight$.pipe(takeUntil(this._destroy$)).subscribe(() => {
        this._switchDatepickerMode();
      });
    });
  }

  ngOnInit() {
    setTimeout(() => (this.datepicker.touchUi = this._isThereFreeSpace()));
  }

  ngOnDestroy() {
    if (!!this._destroy$ && !this._destroy$.closed) {
      this._destroy$.next();
      this._destroy$.complete();
    }
  }

  /** returns true if the amount of free space is not enough to show the datepicker*/
  private _checkWhetherSpaceIsCritical(): boolean {
    const windowHeight = this._getWindowHeight();
    const inputElementRect = this.datepicker._datepickerInput
      .getPopupConnectionElementRef()
      .nativeElement.getBoundingClientRect();
    const inputHeight = inputElementRect.height;
    const freeTopSpace = inputElementRect.top;
    const freeBottomSpace = windowHeight - (freeTopSpace + inputHeight);
    return freeTopSpace < 354 && freeBottomSpace < 354;
  }

  private _switchDatepickerMode() {
    const notEnoughFreeSpace = this._checkWhetherSpaceIsCritical();
    if (notEnoughFreeSpace !== this.datepicker.touchUi) {
      this._zone.run(() => (this.datepicker.touchUi = notEnoughFreeSpace));
    }
  }

  private _getWindowHeight() {
    return window.innerHeight
      ? window.innerHeight
      : document.documentElement.clientHeight || document.body.clientHeight || 0;
  }
}

@Malerba
Copy link

Malerba commented Jul 21, 2018 via email

@mohyeid
Copy link

mohyeid commented Sep 19, 2018

Any progress on this? We are still facing this issue on a 13 inch laptop. I think it may need to consider right or left based on available space. On a 13 inch laptop, top or bottom is always not enough for the datepicker when its right centered in the screen!

https://stackblitz.com/edit/datepicker-position-issue?file=main.ts

datepicker

@julianobrasil
Copy link
Contributor Author

julianobrasil commented Sep 20, 2018

@mohyeid, the PR #11607 is approved and waiting to be merged, and maybe it brings some improvement to this situation, but it won't shift the calendar to left/right (it will be on top of the input). Is it an acceptable solution in your case? (it is in my case)

@mohyeid
Copy link

mohyeid commented Sep 21, 2018

@julianobrasil Yea, that would work fine on Datepicker. I was worried about any other cases where popup would be used and would need to be more smart to go right or left as well to make sure it does not cover any important information. I can not think of a usecase at the moment, so I think your fix will be fine.
In the meantime, thanks for the directive tip. I used it as a temrary fix with a few refactor to dimpplify it. Let me know if you see issues with my changes? Its basically checking if the popover is hidden on the top, so it reset top style to 8px. if its hidden on bottom, it reset the bottom style to 8px.

https://stackblitz.com/edit/datepicker-position-issue-p2xgfz

import { Directive, Self, Host, OnDestroy } from '@angular/core';
import { MatDatepicker } from '@angular/material';
import { takeUntil, delay } from 'rxjs/operators';
import { fromEvent, Subject } from 'rxjs';

@Directive({ selector: '[positionDatepicker]' })
export class PositionDatepickerDirective implements OnDestroy {
 private _destroy$ = new Subject<void>();
  constructor(
    @Self()
    @Host()
    private datepicker: MatDatepicker<any>
  ) {
    if(!this.datepicker.touchUi) {
      this.datepicker.openedStream.pipe(takeUntil(this._destroy$), delay(0)).subscribe(() => {
          this.fixOffScreenPosition();
      });
    }
  }

fixOffScreenPosition(): void {
    const top = this.datepicker._popupRef.hostElement!.firstElementChild!.getBoundingClientRect().top;
    const bottom = this.datepicker._popupRef.hostElement!.firstElementChild!.getBoundingClientRect().bottom;
    if (top < 0) {
      (<any>this.datepicker._popupRef.hostElement!.firstElementChild!).style.top = '8px';
      (<any>this.datepicker._popupRef.hostElement!.firstElementChild!).style.bottom = '';
    } else if (window.innerHeight < bottom) {
      (<any>this.datepicker._popupRef.hostElement!.firstElementChild!).style.bottom = '8px';
      (<any>this.datepicker._popupRef.hostElement!.firstElementChild!).style.top = '';
    }
  }

  ngOnDestroy() {
   if (!!this._destroy$ && !this._destroy$.closed) {
      this._destroy$.next();
      this._destroy$.complete();
    }
  }
}

@julianobrasil
Copy link
Contributor Author

julianobrasil commented Sep 30, 2018

@mohyeid, today I spared some time to test your workaround (basically I tried it out in some situations) and there's a not so good effect when the user scrolls the screen vertically to see the calendar, as it (mat-datepicker) tries to keep the calendar on the screen visible area:

image

@vinomano90
Copy link

@blackblood19, as a workaround you can switch your datepickers to touchUi when it's necessary. I'm using this directive to accomplish this task automatically (if you change the viewport size while the calendar is opened it becomes a little messy, and it's back to normal if you close and open the calendar again => currently I can live with that since most of the time the viewport size is defined only when the browser is opened for the first time):

<mat-datepicker appAutoModeSwitch></mat-datepicker>
@Directive({
  selector: '[appAutoModeSwitch]',
})
export class AppDatepickerSwitchDirective implements OnInit, OnDestroy {
  _isShortScreen = false;

  private _monitorScreenHeight$ = fromEvent(window, 'resize');
  private _destroy$ = new Subject<void>();

  constructor(
    private _zone: NgZone,
    @Self()
    @Host()
    private datepicker: MatDatepicker<any>,
  ) {
    this._zone.runOutsideAngular(() => {
      this._monitorScreenHeight$.pipe(takeUntil(this._destroy$)).subscribe(() => {
        this._switchDatepickerMode();
      });
    });
  }

  ngOnInit() {
    setTimeout(() => (this.datepicker.touchUi = this._isThereFreeSpace()));
  }

  ngOnDestroy() {
    if (!!this._destroy$ && !this._destroy$.closed) {
      this._destroy$.next();
      this._destroy$.complete();
    }
  }

  /** returns true if the amount of free space is not enough to show the datepicker*/
  private _checkWhetherSpaceIsCritical(): boolean {
    const windowHeight = this._getWindowHeight();
    const inputElementRect = this.datepicker._datepickerInput
      .getPopupConnectionElementRef()
      .nativeElement.getBoundingClientRect();
    const inputHeight = inputElementRect.height;
    const freeTopSpace = inputElementRect.top;
    const freeBottomSpace = windowHeight - (freeTopSpace + inputHeight);
    return freeTopSpace < 354 && freeBottomSpace < 354;
  }

  private _switchDatepickerMode() {
    const notEnoughFreeSpace = this._checkWhetherSpaceIsCritical();
    if (notEnoughFreeSpace !== this.datepicker.touchUi) {
      this._zone.run(() => (this.datepicker.touchUi = notEnoughFreeSpace));
    }
  }

  private _getWindowHeight() {
    return window.innerHeight
      ? window.innerHeight
      : document.documentElement.clientHeight || document.body.clientHeight || 0;
  }
}

HI

Property is missing _isThereFreeSpace() can u updated ?

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants