civicrm / org.civicrm.shoreditch

Other
45 stars 59 forks source link

CATL-1927: Fix modal fullscreen height issue #474

Closed deb1990 closed 4 years ago

deb1990 commented 4 years ago

Overview

jQuery UI modals do not take the full height of the screen, when making it fullscreen. But now we want the popups to take full height also. So this PR implements that.

Before

2020-11-03 at 3 10 PM

After

2020-11-03 at 3 09 PM

Technical Details

  1. Added a js/jquery-ui-popup.js file, which

    • Adds a fullscreen class when expand button is pressed.
    • Also assigns the height of the ui-dialog-content dynamically.
      
      var menuHeight = $('#civicrm-menu').outerHeight();
      var titleBarHeight = $el.parent().find('.ui-dialog-titlebar').outerHeight();
      var buttonContainerHeight = $el.parent().find('.ui-dialog-buttonpane').outerHeight();
      var windowHeight = $(window).outerHeight();

    // this is the height we are setting var contentHeight = windowHeight - (titleBarHeight + menuHeight + buttonContainerHeight);

  2. In scss/jquery/overrides/_ui-dialog.scss, added styles specific to fullscreen class, to apply full height.

Comment

Backstop JS tests were not run, because currently we dont have any scenarios written for expanding modals.

swastikpareek commented 4 years ago

@deb1990 : Could we please mention in the technical details why we need to add js/jquery-ui-popup.js? Do you think we should add details how are we doing this Also assigning the height of the ui-dialog-content dynamically ?

Few other things.

  1. I think the overview needs to be refactored to follow PR guidelines
  2. Did we run backstop for this? If yes, please add it to testing section in the PR details as done here