Add breakpoint based modal variations

[sarayourfriend]

Fixes

Fixes #522 by @panchovm
Fixes #1375 by @AetherUnbound

Description

Updates the existing modal implementation to add breakpoint-based variations. It's just style updates except that the mobile version also includes the logo button. I added that because the only place we actually currently use the modal is for the mobile filters and search type switcher and those use the logo button in the header.

This does also wrap the top bar in a slot to make it easier to modify the modal content without having to copy/paste the whole thing like we've done with the VMobileModalContent.

This PR also removes that component 🎉

Testing Instructions

Checkout the branch and run pnpm dev to visit the site locally.

Go to the search results page with a small viewport width and verify that the modal for the search type switcher and the filters still works as expected and that it looks good.

Next run pnpm storybook and open the Modal story in storybook and test out the various breakpoints using Storybook's breakpoints option or by opening the iframe popout and changing your viewport width manually (the second option can give you a sense of how responsive it is).

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the default branch of the repository (main) or a parent feature branch.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added or updated tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[fcoveram]

It looks great so far. I just noticed two things that maybe you are currently working on.

Modal scroll

When I scroll down, the white area where content is placed remains fixed while the internal content moves up and down. I think the best approach is how Unsplash and Rawpixel solved this. Go and search for something since pasting a URL from both sites takes you to the non-modal version of the single result view.

Modal width

The modal widths for breakpoints 2xl and xl work well, but while changing the viewport manually, I notice the width changes as the page view increases or decreases. I think we should keep the modal width fixed until reaching a different breakpoint. In that way, the content placed inside does not move too much and the modal is always center aligned.

In breakpoints lg and md, the modal width is smaller than the Design Libary mockup. For lg it should be 768px while for md should be full width (768px). Below two screenshots of what I am seeing.

Screenshots

lg	md

[sarayourfriend]

Okay, I didn't understand the mock-ups correctly, I will update it, it's just a matter of moving the widths up one breakpoint.

When I scroll down, the white area where content is placed remains fixed while the internal content moves up and down. I think the best approach is how Unsplash and Rawpixel solved this. Go and search for something since pasting a URL from both sites takes you to the non-modal version of the single result view.

How would we implement that behavior with the close button where it is, above the modal? It would scroll out of view 🤔

[fcoveram]

I do not think that being out of view when scrolling down is terrible as it is consistent with all breakpoints and the back to results. On the other hand, the escape button can work for closing the modal.

[obulat]

I really like the clean-up of the Modal components :)

[sarayourfriend]

@panchovm widths are updated, let me know if they're correct now (I'm having a hard time fully understanding the mock-ups and how they relate to the space between the breakpoints, in particular the space between the sm and md breakpoints. I took a guess, but let me know if it makes sense.

I also updated it so that it scrolls the full modal overlay rather than just the modal body, as you requested.

[fcoveram]

Modal behavior

I run dev and it does not look like a modal but like page that keeps the openverse header fixed. Here is a screenshot.

Non-scroll	Scroll

---

Close area

In md I am seeing the close action outside the modal, whereas in sm and xs the close area has the same close area as the content switcher/filters content. The logic you are following is fine as it is consistent across all content displayed as full-size, but the design proposes a "back to results" top-left button.

I am considering requesting an update of Close modal component to have a close area for modals, content switcher and filters, and header scrolled version. Here is a screenshot of how it might look like.

Let me know what you think

[sarayourfriend]

@panchovm Sorry, there is some confusion. This does not implement the "results open as a modal" behavior, it just updates the modal to behave appropriately for each breakpoint (a necessary step for the results as a modal PR).

When we implement the "results as a modal" PR, we will replace the top bar with the "back to search results" as indicated in the designs. This is easily possible now due to some internal implementation changes I made in this PR as well.

Basically this PR sets us up for implementing the search result modal (so long as we can sort out the browser navigation difficulties, but that's a technical implementation detail, not something you need to worry about 😁)

[fcoveram]

Perfect. Thanks for the clarification. The dev work looks great then ✨

[obulat]

Looks nice, and the code is much cleaner.
You cannot open a modal twice, but I guess it's being worked on in #1279?

[sarayourfriend]

@obulat yes, that's going to be a doozy for another PR. I think I will have to re-write the whole dialog composable ☹️ There's some very weird circular event handling that I'm not understanding at all.

[zackkrida]

These look great, and such a clean approach to fixing the issue. I still see a vertical 1px (!!) shift in the logo when opening the menu, which I tried to debug but was unsuccessful. It's a major improvement regardless.