BREAKING CHANGE(web-twig): Turn off scrolling inside ModalDialog by d…

…efault #DS-1184
lmc-eu · May 21, 2024 · f1ef52d · f1ef52d
1 parent dcd2a03
commit f1ef52d
Show file tree

Hide file tree

Showing 9 changed files with 179 additions and 230 deletions.
diff --git a/docs/migrations/web-twig/MIGRATION-v3.md b/docs/migrations/web-twig/MIGRATION-v3.md
@@ -190,6 +190,9 @@ It is possible to re-enable the inside scrolling by adding the `isScrollable` pr
 
 Add `isScrollable` prop to the `ModalDialog` component.
 
+If you use `ScrollView` for scrolling the content of your modal, you must also make the
+`ModalDialog` scrollable by setting the `isScrollable` prop.
+
 ### Modal: ModalDialog Uniform Appearance
 
 The uniform `ModalDialog` appearance replaced the current behavior. Current mobile appearance

diff --git a/packages/web-twig/DEPRECATIONS-v3.md b/packages/web-twig/DEPRECATIONS-v3.md
@@ -59,19 +59,6 @@ Use:
 
 See [`Tooltip` documentation][tooltip-readme] for more details and examples.
 
-### ModalDialog `isScrollable` Prop
-
-The `isScrollable` prop will be set to `false` by default in the next major release and the ModalDialog will be made
-non-scrollable by default. It will be possible to re-enable the inside scrolling by adding the `isScrollable` prop.
-
-#### Migration Guide
-
-Add `isScrollable` prop to the `ModalDialog` component.
-
-#### Migration Guide
-
-Either install newer version of the `spirit-icons` package or add `danger` named icon to your project's icons.
-
 [dropdown-readme]: https://github.com/lmc-eu/spirit-design-system/blob/main/packages/web-twig/src/Resources/components/Dropdown/README.md
 [readme-deprecations]: https://github.com/lmc-eu/spirit-design-system/blob/main/packages/web-twig/README.md#deprecations
 [tooltip-readme]: https://github.com/lmc-eu/spirit-design-system/blob/main/packages/web-twig/src/Resources/components/Tooltip/README.md
diff --git a/packages/web-twig/src/Resources/components/Modal/Modal.stories.twig b/packages/web-twig/src/Resources/components/Modal/Modal.stories.twig
@@ -40,7 +40,7 @@
         }
 
         const toggleScrolling = (selector) => {
-            document.querySelector(selector).classList.toggle('ModalDialog--nonScrollable');
+            document.querySelector(selector).classList.toggle('ModalDialog--scrollable');
         }
     </script>
 

diff --git a/packages/web-twig/src/Resources/components/Modal/ModalDialog.twig b/packages/web-twig/src/Resources/components/Modal/ModalDialog.twig
@@ -3,7 +3,7 @@
 {%- set _elementType = props.elementType | default('article') -%}
 {%- set _isDockedOnMobile = props.isDockedOnMobile | default(false) -%}
 {%- set _isExpandedOnMobile = props.isExpandedOnMobile | default(false) -%}
-{%- set _isScrollable = props.isScrollable ?? true -%}
+{%- set _isScrollable = props.isScrollable | default(false) -%}
 {%- set _maxHeightFromTabletUp = props.maxHeightFromTabletUp | default(null) -%}
 {%- set _preferredHeightOnMobile = props.preferredHeightOnMobile | default(null) -%}
 {%- set _preferredHeightFromTabletUp = props.preferredHeightFromTabletUp | default(null) -%}
@@ -12,7 +12,7 @@
 {%- set _rootClassName = _spiritClassPrefix ~ 'ModalDialog' -%}
 {%- set _rootDockOnMobileClassName = _isDockedOnMobile ? _spiritClassPrefix ~ 'ModalDialog--dockOnMobile' : null -%}
 {%- set _rootExpandOnMobileClassName = _isExpandedOnMobile ? _spiritClassPrefix ~ 'ModalDialog--expandOnMobile' : null -%}
-{%- set _rootScrollableClassName = _isScrollable ? null : _spiritClassPrefix ~ 'ModalDialog--nonScrollable' -%}
+{%- set _rootScrollableClassName = _isScrollable ? _spiritClassPrefix ~ 'ModalDialog--scrollable' : null -%}
 
 {# Miscellaneous #}
 {%- set _styleProps = useStyleProps(props) -%}

diff --git a/packages/web-twig/src/Resources/components/Modal/README.md b/packages/web-twig/src/Resources/components/Modal/README.md
@@ -96,7 +96,7 @@ On mobile screens, Modal can be docked to the bottom of the viewport using the `
 
 #### Expanded Variant
 
-By default the docked dialog on mobile screens shrinks to fit the height of its content
+By default, the docked dialog on mobile screens shrinks to fit the height of its content
 (if smaller than the viewport). Use the `isExpandedOnMobile` option to expand the dialog on mobile.
 
 ```twig
@@ -105,66 +105,26 @@ By default the docked dialog on mobile screens shrinks to fit the height of its
 </ModalDialog>
 ```
 
-### Custom Height
-
-By default, Modal expands to fit the height of its content, as long as it fits the viewport (see [more below](#custom-max-height)).
-You can override this behavior by setting a preferred height using the following options:
-
-- `preferredHeightOnMobile` for mobile screens, and
-- `preferredHeightFromTabletUp` for tablet screens and up.
-
-This is useful for Modals with dynamic content, e.g. a list of items that can be added or removed, or a multistep wizard.
-
-```twig
-<ModalDialog preferredHeightOnMobile="400px" preferredHeightFromTabletUp="500px">
-  …
-</ModalDialog>
-```
-
-👉 Please note the preferred height options are ignored when scrolling inside ModalDialog is
-[turned off](#disable-scrolling-inside-modaldialog).
-
-👉 Please note the custom height values are considered **preferred:** Modal will not expand beyond the viewport height.
-
-### Custom Max Height
-
-The default maximum height of Modal is:
-
-- viewport height minus 1100 spacing on mobile screens, and
-- 600 px on tablet screens and up (shrunk if necessary).
-
-You can use the `maxHeightFromTabletUp` option to override the max height on tablet screens and up:
-
-```twig
-<ModalDialog maxHeightFromTabletUp="700px">
-  …
-</ModalDialog>
-```
-
-👉 Please note the max height is ignored when scrolling inside ModalDialog is [turned off](#disable-scrolling-inside-modaldialog).
-
-👉 Please note the max height on mobile screens is currently not customizable. Let us know if you need this feature! 🙏
-
 ### API
 
-| Name                          | Type                          | Default   | Required | Description                                                                                                                              |
-| ----------------------------- | ----------------------------- | --------- | -------- | ---------------------------------------------------------------------------------------------------------------------------------------- |
-| `accept-charset`              | `string`                      | `null`    | ✕        | `elementType="form"` only: Character encodings to use for form submission (intentionally in kebab-case)                                  |
-| `action`                      | `string`                      | `null`    | ✕        | `elementType="form"` only: URL to use for form submission                                                                                |
-| `autocomplete`                | `string`                      | `null`    | ✕        | `elementType="form"` only: [Automated assistance in filling][autocomplete-attr]                                                          |
-| `elementType`                 | `string`                      | `article` | ✕        | HTML tag to render                                                                                                                       |
-| `enctype`                     | `string`                      | `null`    | ✕        | `elementType="form"` only: Encoding to use for form submission                                                                           |
-| `isDockedOnMobile`            | `bool`                        | `false`   | ✕        | [REQUIRES FEATURE FLAG](#feature-flag-uniform-appearance-on-all-breakpoints): Dock the ModalDialog to the bottom of the screen on mobile |
-| `isExpandedOnMobile`          | `bool`                        | `false`   | ✕        | If the ModalDialog should expand on mobile. Overrides any height defined by `preferredHeightOnMobile`.                                   |
-| `isScrollable`                | `bool`                        | `true`    | ✕        | If the ModalDialog should be scrollable. If set to `false`, the dialog will not scroll and will expand to fit the content.               |
-| `maxHeightFromTabletUp`       | `string`                      | `null`    | ✕        | Max height of the modal. Accepts any valid CSS value.                                                                                    |
-| `method`                      | [`get` \| `post` \| `dialog`] | `null`    | ✕        | `elementType="form"` only: HTTP method to use for form submission                                                                        |
-| `name`                        | `string`                      | `null`    | ✕        | `elementType="form"` only: Name of the form                                                                                              |
-| `novalidate`                  | `void`                        | `null`    | ✕        | `elementType="form"` only: [If the dialog should have validation disabled][novalidate-attr]                                              |
-| `preferredHeightFromTabletUp` | `string`                      | `null`    | ✕        | Preferred height of the modal on tablet and larger. Accepts any valid CSS value.                                                         |
-| `preferredHeightOnMobile`     | `string`                      | `null`    | ✕        | Preferred height of the modal on mobile. Accepts any valid CSS value.                                                                    |
-| `rel`                         | `string`                      | `null`    | ✕        | `elementType="form"` only: Relationship between the current document and the linked resource                                             |
-| `target`                      | `string`                      | `null`    | ✕        | `elementType="form"` only: Browsing context for form submission                                                                          |
+| Name                          | Type                          | Default   | Required | Description                                                                                                                |
+| ----------------------------- | ----------------------------- | --------- | -------- | -------------------------------------------------------------------------------------------------------------------------- |
+| `accept-charset`              | `string`                      | `null`    | ✕        | `elementType="form"` only: Character encodings to use for form submission (intentionally in kebab-case)                    |
+| `action`                      | `string`                      | `null`    | ✕        | `elementType="form"` only: URL to use for form submission                                                                  |
+| `autocomplete`                | `string`                      | `null`    | ✕        | `elementType="form"` only: [Automated assistance in filling][autocomplete-attr]                                            |
+| `elementType`                 | `string`                      | `article` | ✕        | HTML tag to render                                                                                                         |
+| `enctype`                     | `string`                      | `null`    | ✕        | `elementType="form"` only: Encoding to use for form submission                                                             |
+| `isDockedOnMobile`            | `bool`                        | `false`   | ✕        | Dock the ModalDialog to the bottom of the screen on mobile                                                                 |
+| `isExpandedOnMobile`          | `bool`                        | `false`   | ✕        | If the ModalDialog should expand on mobile. Overrides any height defined by `preferredHeightOnMobile`.                     |
+| `isScrollable`                | `bool`                        | `true`    | ✕        | If the ModalDialog should be scrollable. If set to `false`, the dialog will not scroll and will expand to fit the content. |
+| `maxHeightFromTabletUp`       | `string`                      | `null`    | ✕        | Max height of the modal. Accepts any valid CSS value.                                                                      |
+| `method`                      | [`get` \| `post` \| `dialog`] | `null`    | ✕        | `elementType="form"` only: HTTP method to use for form submission                                                          |
+| `name`                        | `string`                      | `null`    | ✕        | `elementType="form"` only: Name of the form                                                                                |
+| `novalidate`                  | `void`                        | `null`    | ✕        | `elementType="form"` only: [If the dialog should have validation disabled][novalidate-attr]                                |
+| `preferredHeightFromTabletUp` | `string`                      | `null`    | ✕        | Preferred height of the modal on tablet and larger. Accepts any valid CSS value.                                           |
+| `preferredHeightOnMobile`     | `string`                      | `null`    | ✕        | Preferred height of the modal on mobile. Accepts any valid CSS value.                                                      |
+| `rel`                         | `string`                      | `null`    | ✕        | `elementType="form"` only: Relationship between the current document and the linked resource                               |
+| `target`                      | `string`                      | `null`    | ✕        | `elementType="form"` only: Browsing context for form submission                                                            |
 
 On top of the API options, the components accept [additional attributes][readme-additional-attributes].
 If you need more control over the styling of a component, you can use [style props][readme-style-props]
@@ -307,40 +267,79 @@ Use our JavaScript plugin to open your Modal, e.g.:
 
 ## Scrolling Long Content
 
-When Modals become too long for the user's viewport or device, they automatically scroll independently of the page itself.
+In case the content is longer than user's viewport or device, the ModalBody will expand to fit the height of its content
+and the whole ModalDialog will scroll.
+
+### Enable Scrolling Inside ModalDialog
+
+Scrolling inside ModalDialog can be turned on by adding the `isScrollable` prop:
+
+```twig
+<ModalDialog isScrollable>
+  …
+</ModalDialog>
+```
 
 ### Scrolling with ScrollView
 
 To make content overflow more obvious to users, you can wrap the ModalBody content in a [ScrollView][scroll-view] that
 takes over the responsibility for scrolling and provides visual overflow decorators, e.g.:
 
 ```twig
-<ScrollView data-spirit-toggle="scrollView" overflowDecorators="both">
-  <ModalBody>
-    …Long content…
-  </ModalBody>
-</ScrollView>
+<ModalDialog isScrollable>
+  …
+  <ScrollView data-spirit-toggle="scrollView" overflowDecorators="both">
+    <ModalBody>
+      …Long content…
+    </ModalBody>
+  </ScrollView>
+  …
+</ModalDialog>
 ```
 
-### Disable Scrolling Inside ModalDialog
+### Custom Height
+
+By default, ModalDialog grows to the height of its content until it reaches the [maximum height](#custom-max-height)
+limit.
+
+You can set a custom preferred height of ModalDialog using a custom property:
+
+- `preferredHeightOnMobile` for mobile screens, and
+- `preferredHeightFromTabletUp` for tablet screens and up.
 
-Scrolling inside ModalDialog can be turned off by setting the `ModalDialog` prop `isScrollable` to `false`:
+This is useful for Modals with dynamic content, e.g. a list of items that can be added or removed, or a multistep wizard.
 
 ```twig
-<ModalDialog isScrollable={ false }>
-  <!-- … -->
+<ModalDialog isScrollable preferredHeightOnMobile="400px" preferredHeightFromTabletUp="500px">
+  …
 </ModalDialog>
 ```
 
-This way, the ModalBody will expand to fit the height of its content and the whole ModalDialog will scroll in case the
-content is longer than user's viewport.
+⚠️ This feature is only available for ModalDialogs with the `isScrollable` prop.
+
+👉 Please note the custom height values are considered **preferred**. Scrollable ModalDialog will never expand beyond
+the viewport height. See the [Custom Max Height](#custom-max-height) section for more information.
+
+### Custom Max Height
+
+The default maximum height of a scrollable ModalDialog is **600 px**, as long as it can fit the viewport.
+
+If the viewport is smaller, scrollable ModalDialog will shrink to fit the viewport. In such case, the ModalDialog height
+will calculate as "viewport height (`100dvh`) minus 1100 spacing".
 
-👉 Please note that this modifier class can produce unexpected results when used in combination with ScrollView.
+You can use the prop `maxHeightFromTabletUp` to override the default maximum height limit on tablet
+screens and up:
+
+```twig
+<ModalDialog isScrollable maxHeightFromTabletUp="700px">
+  …
+</ModalDialog>
+```
 
-#### ⚠️ DEPRECATION NOTICE
+⚠️ This feature is only available for ModalDialogs with the `isScrollable` prop.
 
-The `isScrollable` prop will be set to `false` by default in the next major release and the ModalDialog will be made
-non-scrollable by default. It will be possible to re-enable the inside scrolling by adding the `isScrollable` prop.
+👉 If a [custom height](#custom-height) is set, the custom max height is only applied if it's smaller than the custom
+height.
 
 ## Stacking Modals
 
@@ -413,7 +412,6 @@ Or, feel free to write the controlling script yourself.
 [modal]: https://github.com/lmc-eu/spirit-design-system/tree/main/packages/web/src/scss/components/Modal
 [novalidate-attr]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/form#novalidate
 [readme-additional-attributes]: https://github.com/lmc-eu/spirit-design-system/blob/main/packages/web-twig/README.md#additional-attributes
-[readme-deprecations]: https://github.com/lmc-eu/spirit-design-system/blob/main/packages/web-twig/README.md#deprecations
 [readme-escape-hatches]: https://github.com/lmc-eu/spirit-design-system/blob/main/packages/web-twig/README.md#escape-hatches
 [readme-feature-flags]: https://github.com/lmc-eu/spirit-design-system/blob/main/packages/web/README.md#feature-flags
 [readme-style-props]: https://github.com/lmc-eu/spirit-design-system/blob/main/packages/web-twig/README.md#style-props

diff --git a/packages/web-twig/src/Resources/components/Modal/__tests__/__fixtures__/modalDefault.twig b/packages/web-twig/src/Resources/components/Modal/__tests__/__fixtures__/modalDefault.twig
@@ -43,7 +43,7 @@
         elementType="form"
         isDockedOnMobile
         isExpandedOnMobile
-        isScrollable={ false }
+        isScrollable
         maxHeightFromTabletUp="700px"
         method="dialog"
         preferredHeightOnMobile="400px"

diff --git a/...b-twig/src/Resources/components/Modal/__tests__/__snapshots__/modalDefault.twig.snap.html b/...b-twig/src/Resources/components/Modal/__tests__/__snapshots__/modalDefault.twig.snap.html
@@ -59,7 +59,7 @@ <h2 class="ModalHeader__title" id="modal-example-title">
     <!-- Render with all props -->
 
     <dialog class="Modal Modal--top" id="modal-example-2" aria-labelledby="modal-example-2-title">
-      <form autocomplete="on" method="dialog" class="ModalDialog ModalDialog--dockOnMobile ModalDialog--expandOnMobile ModalDialog--nonScrollable custom-class" style="--modal-max-height-tablet: 700px;--modal-preferred-height-mobile: 400px;--modal-preferred-height-tablet: 500px;outline: 5px solid blue; outline-offset: -5px;">
+      <form autocomplete="on" method="dialog" class="ModalDialog ModalDialog--dockOnMobile ModalDialog--expandOnMobile ModalDialog--scrollable custom-class" style="--modal-max-height-tablet: 700px;--modal-preferred-height-mobile: 400px;--modal-preferred-height-tablet: 500px;outline: 5px solid blue; outline-offset: -5px;">
         <header class="ModalHeader">
           <h2 class="ModalHeader__title" id="modal-example-2-title">
             Title of the Modal