feat(modal): modal implements dialog

.changeset/modal.md

@@ -16,6 +16,19 @@ This release migrates `<pfe-modal>` to LitElement.
 - Deprecates `pfe-modal--header` slot. Use `header`
 - Deprecates `pfe-modal:open` event. Use `open`
 - Deprecates `pfe-modal:close` event. Use `close`
+- Passing an event to `open()` and `toggle()` no longer assigns the trigger element.
+    use `setTrigger(triggerElement)` instead,
+    or set the `trigger` attribute to the id of a trigger element in the same root as the modal.
+
+### HTMLDialogElement Interface
+
+`<pfe-modal>` now implements the `HTMLDialogElement` interface. As such, a number of element APIs have changed:
+
+- renames the private `isOpen` property to `open` and make it public, and make it reflect to the `open` attribute
+- renames the `open()` method to `show()`
+- adds `showModal()` as an alias of `show()`
+- adds a `cancel` event, distinguishing between `close` and `cancel` events
+- adds a `returnValue` property, which can be set by passing a string to `close(returnValue)`
 
 
 See [docs](https://patternflyelements.org/components/modal/) for more info

docs/_data/todos.json

@@ -1 +1 @@
-[{"file":"elements/pfe-accordion/pfe-accordion.ts","tag":"TODO","line":337,"ref":"","text":"use Element#animate"},{"file":"elements/pfe-button/pfe-button.ts","tag":"TODO","line":66,"ref":"","text":"describe the semantic meaning of these states"},{"file":"elements/pfe-clipboard/pfe-clipboard.ts","tag":"TODO","line":147,"ref":"","text":"Remove deprecated `text--success` slot and associated logic in 3.0"},{"file":"elements/pfe-clipboard/pfe-clipboard.ts","tag":"TODO","line":149,"ref":"","text":"Remove deprecated `text` slot and associated logic in 3.0"},{"file":"elements/pfe-dropdown/pfe-dropdown-item.ts","tag":"TODO","line":70,"ref":"","text":"Deprecate - bp"},{"file":"elements/pfe-dropdown/pfe-dropdown.ts","tag":"TODO","line":279,"ref":"","text":"evaluate how expensive it is to use innerHTML in this manor"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":66,"ref":"","text":"This needs a click handler for if the accordion is stuck to the top"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":126,"ref":"","text":"note this in the documentation as a readonly property"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":134,"ref":"","text":"Document this more"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":145,"ref":"","text":"Need to incorporate support for breakpoint customizations i.e., offset=\"@500px: 200, @800px: 150\""},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":374,"ref":"","text":"It seems like the offset is 0 when non-horizontal jumps links are mobile"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":395,"ref":"","text":"remove this when deprecating pfe-navigation, instead set the offset variable in nav cases"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":490,"ref":"","text":"would be good to set the last item as active if the visible nav is below this one"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":522,"ref":"","text":"respond to URL change? Ensure anchor link alignment accounts for sticky nav(s)"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":544,"ref":"","text":"respond to URL change? Ensure anchor link alignment accounts for sticky nav(s)"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":599,"ref":"","text":"Do other items in the stack need to be unstuck?"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":808,"ref":"","text":"Add a keyboard handler when focus is set on the parent via keyboard; should expand"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":815,"ref":"","text":"On page load, if an anchor exists, fire this event"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":1080,"ref":"","text":"should this processing even be happening?"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":1095,"ref":"","text":"The next logic only works for scrolling down; need to reverse for scrolling up"},{"file":"elements/pfe-jump-links/pfe-jump-links-panel.ts","tag":"TODO","line":9,"ref":"","text":"This needs a click handler for if the accordion is stuck to the top"},{"file":"elements/pfe-progress-steps/pfe-progress-steps-item.ts","tag":"TODO","line":156,"ref":"","text":"This needs AT"},{"file":"elements/pfe-tabs/pfe-tab-panel.ts","tag":"TODO","line":31,"ref":"","text":"Should deprecate"}]
+[{"file":"elements/pfe-accordion/pfe-accordion.ts","tag":"TODO","line":337,"ref":"","text":"use Element#animate"},{"file":"elements/pfe-button/pfe-button.ts","tag":"TODO","line":66,"ref":"","text":"describe the semantic meaning of these states"},{"file":"elements/pfe-clipboard/pfe-clipboard.ts","tag":"TODO","line":147,"ref":"","text":"Remove deprecated `text--success` slot and associated logic in 3.0"},{"file":"elements/pfe-clipboard/pfe-clipboard.ts","tag":"TODO","line":149,"ref":"","text":"Remove deprecated `text` slot and associated logic in 3.0"},{"file":"elements/pfe-dropdown/pfe-dropdown-item.ts","tag":"TODO","line":70,"ref":"","text":"Deprecate - bp"},{"file":"elements/pfe-dropdown/pfe-dropdown.ts","tag":"TODO","line":279,"ref":"","text":"evaluate how expensive it is to use innerHTML in this manor"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":66,"ref":"","text":"This needs a click handler for if the accordion is stuck to the top"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":126,"ref":"","text":"note this in the documentation as a readonly property"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":134,"ref":"","text":"Document this more"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":145,"ref":"","text":"Need to incorporate support for breakpoint customizations i.e., offset=\"@500px: 200, @800px: 150\""},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":374,"ref":"","text":"It seems like the offset is 0 when non-horizontal jumps links are mobile"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":395,"ref":"","text":"remove this when deprecating pfe-navigation, instead set the offset variable in nav cases"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":490,"ref":"","text":"would be good to set the last item as active if the visible nav is below this one"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":522,"ref":"","text":"respond to URL change? Ensure anchor link alignment accounts for sticky nav(s)"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":544,"ref":"","text":"respond to URL change? Ensure anchor link alignment accounts for sticky nav(s)"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":599,"ref":"","text":"Do other items in the stack need to be unstuck?"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":808,"ref":"","text":"Add a keyboard handler when focus is set on the parent via keyboard; should expand"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":815,"ref":"","text":"On page load, if an anchor exists, fire this event"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":1080,"ref":"","text":"should this processing even be happening?"},{"file":"elements/pfe-jump-links/pfe-jump-links-nav.ts","tag":"TODO","line":1095,"ref":"","text":"The next logic only works for scrolling down; need to reverse for scrolling up"},{"file":"elements/pfe-jump-links/pfe-jump-links-panel.ts","tag":"TODO","line":9,"ref":"","text":"This needs a click handler for if the accordion is stuck to the top"},{"file":"elements/pfe-modal/pfe-modal.ts","tag":"TODO","line":187,"ref":"","text":"in non-modal case, toggle the dialog"},{"file":"elements/pfe-progress-steps/pfe-progress-steps-item.ts","tag":"TODO","line":156,"ref":"","text":"This needs AT"},{"file":"elements/pfe-tabs/pfe-tab-panel.ts","tag":"TODO","line":31,"ref":"","text":"Should deprecate"}]

elements/pfe-modal/README.md

@@ -9,8 +9,8 @@
 ### With a trigger
 ```html
 <pfe-modal>
-  <button slot="pfe-modal--trigger">Open modal</button>
-  <h2 slot="pfe-modal--header">Modal with a header</h2>
+  <button slot="trigger">Open modal</button>
+  <h2 slot="header">Modal with a header</h2>
   <p>Lorem ipsum dolor sit amet, <a href="#foo">consectetur adipisicing</a> elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
   <pfe-cta>
     <a href="#bar">Learn more</a>
@@ -21,7 +21,7 @@
 ### Without a trigger
 ```html
 <pfe-modal>
-  <h2 slot="pfe-modal--header">Modal with a header</h2>
+  <h2 slot="header">Modal with a header</h2>
   <p>Lorem ipsum dolor sit amet, <a href="#foo">consectetur adipisicing</a> elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
   <pfe-cta>
     <a href="#bar">Learn more</a>
@@ -34,8 +34,8 @@ The `width` attribute controls the width of the modal window. There are three op
 
 ```html
 <pfe-modal width="small">
-  <button slot="pfe-modal--trigger">Open a small modal</button>
-  <h2 slot="pfe-modal--header">Modal with a header</h2>
+  <button slot="trigger">Open a small modal</button>
+  <h2 slot="header">Modal with a header</h2>
   <p>Lorem ipsum dolor sit amet, <a href="#foo">consectetur adipisicing</a> elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
   <pfe-cta>
     <a href="#bar">Learn more</a>
@@ -113,10 +113,3 @@ From the PFElements root directory, run:
 
     npm start
 
-## Code style
-
-Modal (and all PFElements) use [Prettier][prettier] to auto-format JS and JSON. The style rules get applied when you commit a change. If you choose to, you can [integrate your editor][prettier-ed] with Prettier to have the style rules applied on every save.
-
-[prettier]: https://github.com/prettier/prettier/
-[prettier-ed]: https://github.com/prettier/prettier/#editor-integration
-[web-component-tester]: https://github.com/Polymer/web-component-tester

elements/pfe-modal/demo/demo.css

@@ -3,3 +3,8 @@
   --pfe-modal--MaxWidth: 800px;
   --pfe-modal--MaxHeight: 800px;
 }
+
+#external-lots::part(body) {
+  display: flex;
+  gap: 24px;
+}

elements/pfe-modal/demo/pfe-modal.html

@@ -58,9 +58,9 @@ <h2>Modal: No headings</h2>
   </pfe-modal>
 </pfe-band>
 
-<pfe-band color="lightest">
+<pfe-band id="external-lots" color="lightest">
   <pfe-card color="darkest">
-    <h2 slot="pfe-card--header">Modal: External trigger</h2>
+    <h2 slot="header">Modal: External trigger</h2>
     <pfe-cta><button id="custom-trigger">Custom open modal</button></pfe-cta>
     <pfe-modal id="custom-modal">
       <h2 slot="header">Custom open modal</h2>
@@ -75,8 +75,24 @@ <h2 slot="header">Custom open modal</h2>
     </pfe-modal>
   </pfe-card>
 
+  <pfe-card>
+    <h2 slot="header">Modal: External trigger By Id</h2>
+    <pfe-cta><button id="id-trigger">Custom open modal</button></pfe-cta>
+    <pfe-modal id="custom-modal" trigger="id-trigger">
+      <h2 slot="header">Trigger by ID</h2>
+      <p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et
+        dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex
+        ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat
+        nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit
+        anim id est laborum.</p>
+      <pfe-cta>
+        <a href="#bar">Learn more</a>
+      </pfe-cta>
+    </pfe-modal>
+  </pfe-card>
+
   <pfe-card color="complement">
-    <h2 slot="pfe-card--header">Modal: a lot of content</h2>
+    <h2 slot="header">Modal: a lot of content</h2>
     <pfe-modal context="dark" id="testit">
       <pfe-cta slot="trigger"><button>Open modal</button></pfe-cta>
       <h2 slot="header">Modal with a header with a super duper long title and a lot of content</h2>

elements/pfe-modal/demo/pfe-modal.js

@@ -1,9 +1,10 @@
 import '@patternfly/pfe-band';
+import '@patternfly/pfe-card';
 import '@patternfly/pfe-cta';
 import '@patternfly/pfe-modal';
 
 const root = document.querySelector('[data-demo="pfe-modal"]')?.shadowRoot ?? document;
 const trigger = root.querySelector('#custom-trigger');
-const modal = root.querySelector('#custom-modal');
+const customTriggerModal = root.querySelector('#custom-modal');
 
-trigger.addEventListener('click', event => modal.open(event));
+customTriggerModal.setTrigger(trigger);

elements/pfe-modal/docs/index.md

@@ -14,6 +14,7 @@
 {% band header="Usage" %}
   ### With a trigger
   The `trigger` slot can be used with a trigger element, like a button, to provide a mechanism to open a modal without any additional JavaScript.
+
   ```html
   <pfe-modal>
     <button slot="trigger">Open modal</button>
@@ -37,6 +38,7 @@
     </pfe-cta>
   </pfe-modal>
   ```
+
 {% endband %}
 
 {% renderSlots %}{% endrenderSlots %}