Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modal popover for report and license definition #3582

Closed
fcoveram opened this issue Dec 22, 2023 · 5 comments · Fixed by #4800
Closed

Modal popover for report and license definition #3582

fcoveram opened this issue Dec 22, 2023 · 5 comments · Fixed by #4800
Assignees
Labels
🖼️ aspect: design Concerns related to design design: ready Issue with a mockup ready for implementation ✨ goal: improvement Improvement to an existing user-facing feature 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend

Comments

@fcoveram
Copy link
Contributor

fcoveram commented Dec 22, 2023

Problem

As reported in #569, reporting content is currently made from a popover that looks broken on small screens. Based on the recent Design Library updates, we can improve this, and also the license definition popover, by reusing the component merged in #2860

Issues related

Proposal

Search results page

Since the license description element is toggled from the drawer in small screens, I have two ideas that I would like your thoughts on.

v1. Replace drawer with the license

In this first version, once you click on the license’s question icon, the drawer disappears and is replaced by the license popover. You can see how the results grid is visible behind the modal layer.

Drawer  v1

v2. Push the drawer behind modal opaque layer

The second idea is to push the drawer behind the modal layer to avoid losing the context of where users displayed the info from. See how the drawer remains behind the dark layer.

I am more drawn to this version as contexts in modals play a key role.

Drawer  v2

From LG and beyond

On bigger screens, the license description popover is displayed as in the current flow.

Results with modal popover

Single result page

In the single result page, the popover shows up as in the current flow.

Single result page with modal popover

Popovers

As described in #2860, the following specs are still valid:

  • Popover is centered, full width, and has a max value of 400px.
  • On xs , horizontal margins 24px

However, I noticed that we did not consider a scrollable popover for those cases where its height overflows the device screen. For that case, I tried the following:

Scrollable popover behavior

The spacings mirror the ones applied to the current component, but instead of wrapping the content with a single element, the popover's header is separated to keep it sticky when scrolling down. Below a screenshot of dev mode.

Popover with notes in header

Mockups

Figma file: Modal popover for report and license definition

@fcoveram fcoveram added 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work ✨ goal: improvement Improvement to an existing user-facing feature 🖼️ aspect: design Concerns related to design labels Dec 22, 2023
@openverse-bot
Copy link
Collaborator

Subscribe to Label Action

cc @WordPress/gutenberg-design, @WordPress/openverse

This issue or pull request has been labeled: "🖼️ aspect: design"

Thus the following users have been cc'd because of the following labels:

  • WordPress/gutenberg-design: 🖼️ aspect: design
  • WordPress/openverse: 🖼️ aspect: design

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@openverse-bot openverse-bot moved this to 📋 Backlog in Openverse Backlog Dec 22, 2023
@fcoveram fcoveram added the design: in discussion Issue has a design that needs feedback label Dec 22, 2023
@obulat obulat added 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend and removed 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work labels Dec 25, 2023
@sarayourfriend
Copy link
Collaborator

FWIW I agree that the additional context in v2 makes a lot of sense to me. As a bonus, it would also reduce the number of visual changes happening on the screen as you click into each license popover and back to the filters.

@fcoveram
Copy link
Contributor Author

fcoveram commented May 20, 2024

The mockups for v2 (Push the drawer behind the modal opaque layer) were updated with the latest changes and are ready for dev.

@fcoveram fcoveram added design: ready Issue with a mockup ready for implementation and removed design: in discussion Issue has a design that needs feedback labels May 20, 2024
@obulat obulat moved this from 📋 Backlog to 📅 To Do in Openverse Backlog May 20, 2024
@zackkrida
Copy link
Member

This looks great, thank you @fcoveram

@obulat obulat self-assigned this Jun 14, 2024
@obulat obulat added the ⛔ status: blocked Blocked & therefore, not ready for work label Jul 22, 2024
@obulat
Copy link
Contributor

obulat commented Jul 22, 2024

Blocked by the code freeze during Nuxt 3 migration.

@openverse-bot openverse-bot moved this from 📅 To Do to ⛔ Blocked in Openverse Backlog Jul 22, 2024
@obulat obulat removed the ⛔ status: blocked Blocked & therefore, not ready for work label Jul 31, 2024
@openverse-bot openverse-bot moved this from ⛔ Blocked to 📋 Backlog in Openverse Backlog Jul 31, 2024
@obulat obulat moved this from 📋 Backlog to 🏗 In Progress in Openverse Backlog Aug 22, 2024
@openverse-bot openverse-bot moved this from 🏗 In Progress to ✅ Done in Openverse Backlog Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🖼️ aspect: design Concerns related to design design: ready Issue with a mockup ready for implementation ✨ goal: improvement Improvement to an existing user-facing feature 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants