Make Dialog/Modal Generic

[slkennedy]

Should we consider reworking these two components to be one component that is a container and can accept a variety of content components. Note: if this approach is done then we will need to add a heading for the container component since it's a sectioning element, it can be hidden with sr-only and could be a generic label like "promotional dialog".

[JSpell]

I am going to say yes, I think we should do this. Split them out so that you have a component that encompasses the background cover & a containing/section element to add content to. Then there should be logic to control how the background & containing element function depending on the content.

[slkennedy]

I'm leaning this way too. This is similar to other components we have that work as containers only - willow-page-header, willow-page-footer.

If we go this route, do we need to build 2 new components - overlay and something for popup content?

Rough Example: No deep thought was given to this at this time.

<!-- Overlay -->
<div class="willow-overlay">
    <!-- Insert other components here -->
</div>

<!-- Dialog Modal Popup Content Guy -->
<section class="willow-popup/dialog-and-modal">
    <h1 class="willow-dialog__heading">Heading Text</h1>
    <div class="willow-dialog__content">
        <!-- insert content components here-->
    </div>
    <ul class="willow-dialog__actions">
        <li class="willow-dialog__action"><!-- insert action component here --></li>
    </ul>
</section>

[Keale2]

I agree that the overlay should be broken out into its own component. I think it should be a div as in @slkennedy's comment above, and it shouldn't have any semantic responsibility for whatever is placed within it. I think its state should be controlled by a data attribute similar to willow-page-header. https://github.com/unumux/willow/tree/master/components/page-header#states

I propose data-open. When data-open is true, the dialog should be visible. If data-open has any other value, or if it isn't present, the overlay should not be visible.

Does the second component need to be a new component? If yes, is it something that always needs to be used in an overlay, or is it a common layout with optional actions that we could also use within the content of a page, within a sidebar, etc.? Should we consider a standalone willow-actions component (or something similar)?

[JSpell]

Ideas - https://codepen.io/JSpell/pen/gjavVv

[JSpell]

After our review meeting today, one of the coding take aways was to remove the outer .willow-overlay container in favor of a pseudo element to accomplish the same thing. Leaving the anticipated HTML to look like

<div class="willow-popup" data-popup-styling="default">
    <button class="willow-popup__close">
        <span class="willow-icon willow-icon--close-inverse"></span>
    </button>
    <div class="willow-popup__content"> 
            <!-- Your content here! -->
    </div>
</div>

A very quick mockup of this proves to be problematic. The pseudo element will not escape its parent element when the parent element is positioned absolute & has a transform applied to it. Therefore, using the pseudo element approach means that the .willow-popup element has to be what is set to fixed & positioned to cover the screen & then the pseudo element, which is a child of .willow-popup, fills its parent container.

Also this would force us to move the "chrome" styling to the .willow-popup__content element as discussed. Therefore leaving the close button outside of the padded & styled area. At a first glance, this would necessitate very complex & brittle styling to position & keep the close button in the top right of the content area.

That leads me to the following options for ease of positioning for the close button

1) Wrap the button & content elements in a containing element -- this seems to defeat the purpose of removing the outer overlay elements b/c we're just swapping one for the other.

2) Move the close button into the content element, which seems problematic in it's own right, but it may be the only viable option.

Something else that we didn't discuss in the meeting, & we may not be at this point yet but I will leave it here for future reference, is how are we going to handle vertical content that exceeds the max height (currently viewport height) of the container. Do we scroll the entire container? Do we only scroll the content with in the container and keep the close button visible?

[slkennedy]

Good points @JSpell.

I worked up a new example starting from your work and adding some input from @chuckhendo and @Keale2. I captured our questions in the HTML panel and I'm pulling in @belladactyl to help search for possible answers to those questions.

https://codepen.io/slkennedy/pen/oMPByo?editors=1100

@rebekahpadula @townivan @dbay24

[townivan]

Opinion feedback:

1. Controls: Can popup have multiple controls such as close, fullwidth, something else?

Just close.

2. Need to know if the controls should be positioned next to content (via float:right) or if they should be a block causing the content to start below the controls

Controls should be block causing the content to start below the controls

3. Long Content: How do we handle scrolling on long content in the popup?

Scroll the page. Allow user to exit popup without needing to scroll back to the top by tapping in the overlay area that is not part of the popup container

[JSpell]

I agree with @townivan on questions 1 & 2. I am still on the fence about what/where should scroll.

[Keale2]

I agree on all 3. Scrolling only within the content section of the popup requires the mouse to be positioned correctly before scrolling. Scrolling the page also has the added benefit of being able to tell that the content is longer than what is currently in the viewport, because the content area touches the bottom of the viewport (no margin below it where you can see the transparent overlay part).

[rebekahpadula]

I agree on all three too, wasn't sure about 3 at first but having to position your mouse before scrolling would be super annoying.

[slkennedy]

Started a comparison of similar components from other libraries.

I agree with @townivan on all 3 as well.

[slkennedy]

Tweaked the codepen a bit to go with the feedback regarding the 3 questions (referenced above). And added styling to make the content/container fullscreen on smaller devices - this is something in the current public site design so pulling from that.

https://codepen.io/slkennedy/pen/oMPByo?editors=0100