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: exposeAsideWhen doesn't not resize the view body #4898

Closed
5im0n opened this issue Jan 4, 2016 · 4 comments
Closed

bug: exposeAsideWhen doesn't not resize the view body #4898

5im0n opened this issue Jan 4, 2016 · 4 comments
Milestone

Comments

@5im0n
Copy link
Contributor

5im0n commented Jan 4, 2016

Type: bug

Platform: all

This bug is essentially linked to the commit 8187367.

When expose-aside-when="large", the onResize is trigger with the $window.matchMedia function.
However on large device (iPad, Nexus 9) the side menu is open on portrait, when we rotate the tablet in landscape, the resize function is not tigger. So a blank zone appears

In browser when we resize the windows the same problem appears.

So what is the best solutions:
@msalcala11 ?

  • Add window.addEventListener('orientationchange', onOrientationChange); ?
  • Revert the commit 8187367 ?
  • Anything else ?

Samples to illustrate the issue:

portrait
landscapee

@5im0n 5im0n changed the title exposeAsideWhen doesn't not resize the view body bug: exposeAsideWhen doesn't not resize the view body Jan 4, 2016
@msalcala11
Copy link

Thanks for the detailed explanation and screenshots, @5im0n! The problem with the previous implementation was that the resize handler was triggered too often, even when the screen dimensions did not change (such as when tapping a popover), causing the side menu to close for no reason. But looks like my commit went too far, and now the resize handler is not triggered often enough. I think the correct solution is somewhere in the middle. Here's a first pass at a fix (let me know if it resolves your issue):

IonicModule.directive('exposeAsideWhen', ['$window', function($window) {
  return {
    restrict: 'A',
    require: '^ionSideMenus',
    link: function($scope, $element, $attr, sideMenuCtrl) {

      var prevInnerWidth = $window.innerWidth;
      var prevInnerHeight = $window.innerHeight;

      ionic.on('resize', function(){
        if(prevInnerWidth === $window.innerWidth && prevInnerHeight === $window.innerHeight){
          return;
        }
        prevInnerWidth = $window.innerWidth;
        prevInnerHeight = $window.innerHeight;
        onResize();
      }, $window);

      function checkAsideExpose() {
        var mq = $attr.exposeAsideWhen == 'large' ? '(min-width:768px)' : $attr.exposeAsideWhen;
        sideMenuCtrl.exposeAside($window.matchMedia(mq).matches);
        sideMenuCtrl.activeAsideResizing(false);
      }

      function onResize() {
        sideMenuCtrl.activeAsideResizing(true);
        debouncedCheck();
      }

      var debouncedCheck = ionic.debounce(function() {
        $scope.$apply(checkAsideExpose);
      }, 300, false);

      $scope.$evalAsync(checkAsideExpose);
    }
  };
}]);

@5im0n
Copy link
Contributor Author

5im0n commented Jan 5, 2016

@msalcala11 Sounds great for me 👍

Wich this solution the resize is not triggered to often but enough when $window width or height change.
Have you time to make a pull request, or I can do it ?
Would be nice to include this fix is a next release.

@msalcala11
Copy link

@5im0n Glad to hear it worked for you 👍
Yeah, if you could make a pull request, that would be awesome!

@mlynch
Copy link
Contributor

mlynch commented Jan 8, 2016

Closed with #4908 merged. Thanks everyone!

@mlynch mlynch closed this as completed Jan 8, 2016
@mlynch mlynch added this to the 1.2.5 milestone Jan 8, 2016
@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants