Use a modal for license explanation (#4800)

* Use a modal for license explanation Signed-off-by: Olga Bulat <[email protected]> * Fix content report padding Signed-off-by: Olga Bulat <[email protected]> * Add id to unit tests Signed-off-by: Olga Bulat <[email protected]> * Make modal scrollable when overflowing Signed-off-by: Olga Bulat <[email protected]> * Abstract the composable to handle popovers Signed-off-by: Olga Bulat <[email protected]> * Use constants for ids Signed-off-by: Olga Bulat <[email protected]> * Remove negative margin from external search form Signed-off-by: Olga Bulat <[email protected]> * Update external sources modal snapshots Signed-off-by: Olga Bulat <[email protected]> * Add doc comments Signed-off-by: Olga Bulat <[email protected]> --------- Signed-off-by: Olga Bulat <[email protected]>
WordPress · Aug 24, 2024 · bc81c65 · bc81c65
1 parent 8569484
commit bc81c65
Show file tree

Hide file tree

Showing 33 changed files with 200 additions and 73 deletions.
diff --git a/frontend/src/components/VContentReport/VContentReportModal.vue b/frontend/src/components/VContentReport/VContentReportModal.vue
@@ -2,6 +2,7 @@
 import { ref } from "vue"
 
 import { WIP } from "~/constants/content-report"
+import { CONTENT_REPORT_DIALOG } from "~/constants/dialogs"
 import { useContentReport } from "~/composables/use-content-report"
 import type { AudioDetail, ImageDetail } from "~/types/media"
 
@@ -36,6 +37,7 @@ const close = () => {
 
 <template>
   <VModal
+    :id="CONTENT_REPORT_DIALOG"
     ref="modalRef"
     :label="$t('mediaDetails.contentReport.long')"
     :hide-on-click-outside="true"
@@ -51,7 +53,7 @@ const close = () => {
     <template #default>
       <VContentReportForm
         ref="contentReportFormRef"
-        class="p-7 pt-0 sm:p-9"
+        class="p-7 pt-0 sm:p-9 sm:pt-0"
         :media="media"
         :status="status"
         :allow-cancel="true"

diff --git a/frontend/src/components/VContentSwitcher/VSearchTypePopover.vue b/frontend/src/components/VContentSwitcher/VSearchTypePopover.vue
@@ -3,6 +3,7 @@ import { computed, ref } from "vue"
 
 import useSearchType from "~/composables/use-search-type"
 
+import { SEARCH_TYPES_DIALOG } from "~/constants/dialogs"
 import type { SearchType } from "~/constants/media"
 
 import VPopover from "~/components/VPopover/VPopover.vue"
@@ -38,6 +39,7 @@ const handleSelect = (searchType: SearchType) => {
 
 <template>
   <VPopover
+    :id="SEARCH_TYPES_DIALOG"
     ref="contentMenuPopover"
     :label="$t('searchType.label')"
     placement="bottom-end"

diff --git a/frontend/src/components/VExternalSearch/VExternalSearchForm.vue b/frontend/src/components/VExternalSearch/VExternalSearchForm.vue
@@ -11,6 +11,8 @@ import { useMediaStore } from "~/stores/media"
 
 import { useExternalSources } from "~/composables/use-external-sources"
 
+import { EXTERNAL_SEARCH_DIALOG } from "~/constants/dialogs"
+
 import VExternalSourceList from "~/components/VExternalSearch/VExternalSourceList.vue"
 import VButton from "~/components/VButton.vue"
 import VIcon from "~/components/VIcon/VIcon.vue"
@@ -58,6 +60,7 @@ const isMd = computed(() => uiStore.isBreakpoint("md"))
     data-testid="external-sources-form"
   >
     <VModal
+      :id="EXTERNAL_SEARCH_DIALOG"
       variant="centered"
       :hide-on-click-outside="true"
       labelled-by="external-sources-button"
@@ -100,10 +103,7 @@ const isMd = computed(() => uiStore.isBreakpoint("md"))
           {{ $t("externalSources.title") }}
         </h2>
       </template>
-      <VExternalSourceList
-        class="-mt-3 flex flex-col"
-        :search-term="searchTerm"
-      />
+      <VExternalSourceList class="flex flex-col" :search-term="searchTerm" />
     </VModal>
   </section>
 </template>
diff --git a/frontend/src/components/VFilters/VFilterChecklist.vue b/frontend/src/components/VFilters/VFilterChecklist.vue
@@ -4,18 +4,16 @@ import { useI18n } from "#imports"
 import { useSearchStore } from "~/stores/search"
 
 import type { FilterItem, FilterCategory } from "~/constants/filters"
-
 import type { License } from "~/constants/license"
 
 import { getElements } from "~/utils/license"
 
 import VButton from "~/components/VButton.vue"
 import VCheckbox from "~/components/VCheckbox/VCheckbox.vue"
 import VIcon from "~/components/VIcon/VIcon.vue"
-import VIconButton from "~/components/VIconButton/VIconButton.vue"
 import VLicense from "~/components/VLicense/VLicense.vue"
 import VLicenseExplanation from "~/components/VFilters/VLicenseExplanation.vue"
-import VPopover from "~/components/VPopover/VPopover.vue"
+import VIconButton from "~/components/VIconButton/VIconButton.vue"
 
 type toggleFilterPayload = {
   filterType: FilterCategory
@@ -69,6 +67,14 @@ const isLicense = (code: string): code is License => {
   // Quick check that also prevents "`code` is declared but its value is never read" warning.
   return !!code && props.filterType === "licenses"
 }
+
+const getTitle = (code: string) => {
+  return isLicense(code)
+    ? t("filters.licenseExplanation.licenseDefinition")
+    : t("filters.licenseExplanation.markDefinition", {
+        mark: code.toUpperCase(),
+      })
+}
 </script>
 
 <template>
@@ -93,11 +99,12 @@ const isLicense = (code: string): code is License => {
       </VCheckbox>
 
       <!-- License explanation -->
-      <VPopover
+      <VModal
         v-if="isLicense(item.code)"
-        strategy="fixed"
+        :id="item.code"
+        variant="centered"
+        :hide-on-click-outside="true"
         :label="$t('browsePage.aria.licenseExplanation')"
-        :trap-focus="false"
       >
         <template #trigger="{ a11yProps }">
           <VButton
@@ -110,20 +117,24 @@ const isLicense = (code: string): code is License => {
             <VIcon name="help" />
           </VButton>
         </template>
-        <template #default="{ close }">
-          <div class="relative">
-            <VIconButton
-              :label="getLicenseExplanationCloseAria(item.code)"
-              :icon-props="{ name: 'close' }"
-              variant="transparent-gray"
-              size="small"
-              class="!absolute end-1 top-1"
-              @click="close"
-            />
-            <VLicenseExplanation :license="item.code" />
-          </div>
+        <template #title>
+          <h5 class="text-base font-semibold">
+            {{ getTitle(item.code) }}
+          </h5>
+        </template>
+        <template #close-button="{ close }">
+          <VIconButton
+            :label="getLicenseExplanationCloseAria(item.code)"
+            :icon-props="{ name: 'close' }"
+            variant="transparent-gray"
+            size="small"
+            @click="close"
+          />
+        </template>
+        <template #default>
+          <VLicenseExplanation :license="item.code" />
         </template>
-      </VPopover>
+      </VModal>
     </div>
   </fieldset>
 </template>
diff --git a/frontend/src/components/VFilters/VLicenseExplanation.vue b/frontend/src/components/VFilters/VLicenseExplanation.vue
@@ -19,24 +19,10 @@ defineProps<{
 </script>
 
 <template>
-  <div class="license-explanation w-70 max-w-xs p-6">
-    <h5 class="text-base font-semibold">
-      <template v-if="isLicense(license)">{{
-        $t("filters.licenseExplanation.licenseDefinition")
-      }}</template>
-      <template v-else>{{
-        $t("filters.licenseExplanation.markDefinition", {
-          mark: license.toUpperCase(),
-        })
-      }}</template>
-    </h5>
-
-    <VLicenseElements
-      v-if="license"
-      size="small"
-      class="my-4"
-      :license="license"
-    />
+  <div
+    class="license-explanation flex flex-col gap-y-4 p-6 pt-0 sm:p-9 sm:pt-0"
+  >
+    <VLicenseElements size="small" :license="license" />
 
     <i18n-t
       scope="global"

diff --git a/frontend/src/components/VHeader/VHeaderDesktop.vue b/frontend/src/components/VHeader/VHeaderDesktop.vue
@@ -1,4 +1,7 @@
 <script setup lang="ts">
+/**
+ * The desktop search header.
+ */
 import { useNuxtApp } from "#imports"
 
 import { computed, inject, ref } from "vue"
@@ -22,10 +25,6 @@ import VSearchTypePopover from "~/components/VContentSwitcher/VSearchTypePopover
 
 import type { Ref } from "vue"
 
-/**
- * The desktop search header.
- */
-
 const filterButtonRef = ref<InstanceType<typeof VFilterButton> | null>(null)
 const searchBarRef = ref<InstanceType<typeof VSearchBar> | null>(null)
 

diff --git a/frontend/src/components/VHeader/VHeaderInternal.vue b/frontend/src/components/VHeader/VHeaderInternal.vue
@@ -8,6 +8,8 @@ import { useHydrating } from "~/composables/use-hydrating"
 
 import { useUiStore } from "~/stores/ui"
 
+import { PAGES_DIALOG } from "~/constants/dialogs"
+
 import VHomeLink from "~/components/VHeader/VHomeLink.vue"
 import VPageLinks from "~/components/VHeader/VPageLinks.vue"
 import VModalContent from "~/components/VModal/VModalContent.vue"
@@ -22,10 +24,7 @@ const emit = defineEmits<{
 
 const menuButtonRef = ref<InstanceType<typeof VIconButton> | null>(null)
 const nodeRef = ref<HTMLElement | null>(null)
-const modalContentRef = ref<{
-  $el: HTMLElement
-  deactivateFocusTrap: () => void
-} | null>(null)
+const modalContentRef = ref<InstanceType<typeof VModalContent> | null>(null)
 
 const uiStore = useUiStore()
 
@@ -55,6 +54,7 @@ const {
   onTriggerClick: internalOnTriggerClick,
   triggerA11yProps,
 } = useDialogControl({
+  id: PAGES_DIALOG,
   visibleRef: isModalVisible,
   nodeRef,
   lockBodyScroll,
@@ -105,6 +105,7 @@ watch(route, () => {
       <template v-if="triggerElement">
         <VPopoverContent
           v-if="isSm"
+          :id="PAGES_DIALOG"
           z-index="popover"
           :hide="closePageMenu"
           :visible="isModalVisible"
@@ -121,6 +122,7 @@ watch(route, () => {
         </VPopoverContent>
         <VModalContent
           v-else-if="!isSm"
+          :id="PAGES_DIALOG"
           ref="modalContentRef"
           aria-labelledby="menu-button"
           :hide="closePageMenu"

diff --git a/frontend/src/components/VHeader/VHeaderMobile/VContentSettingsModalContent.vue b/frontend/src/components/VHeader/VHeaderMobile/VContentSettingsModalContent.vue
@@ -7,6 +7,8 @@ import useSearchType from "~/composables/use-search-type"
 
 import { SearchType } from "~/constants/media"
 
+import { CONTENT_SETTINGS_DIALOG } from "~/constants/dialogs"
+
 import VButton from "~/components/VButton.vue"
 import VFilterTab from "~/components/VHeader/VHeaderMobile/VFilterTab.vue"
 import VIcon from "~/components/VIcon/VIcon.vue"
@@ -42,9 +44,7 @@ const props = withDefaults(
   }
 )
 
-defineEmits<{
-  select: [SearchType]
-}>()
+defineEmits<{ select: [SearchType] }>()
 
 const searchStore = useSearchStore()
 const content = useSearchType()
@@ -70,6 +70,7 @@ const clearFilters = () => {
 
 <template>
   <VModalContent
+    :id="CONTENT_SETTINGS_DIALOG"
     :aria-label="$t('header.aria.menu')"
     :hide-on-click-outside="true"
     :hide="close"

diff --git a/frontend/src/components/VHeader/VHeaderMobile/VHeaderMobile.vue b/frontend/src/components/VHeader/VHeaderMobile/VHeaderMobile.vue
@@ -18,6 +18,10 @@ import { useRecentSearches } from "~/composables/use-recent-searches"
 import { useMediaStore } from "~/stores/media"
 import { useSearchStore } from "~/stores/search"
 
+import {
+  CONTENT_SETTINGS_DIALOG,
+  RECENT_SEARCHES_DIALOG,
+} from "~/constants/dialogs"
 import { skipToContentTargetId } from "~/constants/window"
 
 import VLogoButton from "~/components/VHeader/VLogoButton.vue"
@@ -219,6 +223,7 @@ const {
   onTriggerClick: toggleContentSettings,
   triggerA11yProps,
 } = useDialogControl({
+  id: CONTENT_SETTINGS_DIALOG,
   visibleRef: contentSettingsOpen,
   nodeRef: headerRef,
   lockBodyScroll: true,
@@ -352,6 +357,7 @@ const handleTab = (
     </form>
     <VModalContent
       v-if="isRecentVisible"
+      :id="RECENT_SEARCHES_DIALOG"
       :visible="true"
       :hide="deactivate"
       :trigger-element="searchInputRef"

diff --git a/frontend/src/components/VHomepageContent.vue b/frontend/src/components/VHomepageContent.vue
@@ -9,6 +9,8 @@ import { useDialogControl } from "~/composables/use-dialog-control"
 
 import { useUiStore } from "~/stores/ui"
 
+import { HOMEPAGE_CONTENT_SETTINGS_DIALOG } from "~/constants/dialogs"
+
 import VContentSettingsModalContent from "~/components/VHeader/VHeaderMobile/VContentSettingsModalContent.vue"
 import VLink from "~/components/VLink.vue"
 import VPopoverContent from "~/components/VPopover/VPopoverContent.vue"
@@ -68,6 +70,7 @@ const {
   onTriggerClick,
   triggerA11yProps,
 } = useDialogControl({
+  id: HOMEPAGE_CONTENT_SETTINGS_DIALOG,
   visibleRef: isContentSwitcherVisible,
   nodeRef,
   lockBodyScroll,
@@ -108,6 +111,7 @@ const {
       <template v-if="triggerElement">
         <VPopoverContent
           v-if="isLg"
+          :id="HOMEPAGE_CONTENT_SETTINGS_DIALOG"
           z-index="popover"
           :hide="closeContentSwitcher"
           :trap-focus="false"
@@ -124,6 +128,7 @@ const {
 
         <VContentSettingsModalContent
           v-else
+          :id="HOMEPAGE_CONTENT_SETTINGS_DIALOG"
           aria-labelledby="search-type-button"
           :close="closeContentSwitcher"
           :visible="isContentSwitcherVisible"

diff --git a/frontend/src/components/VLicense/VLicenseElements.vue b/frontend/src/components/VLicense/VLicenseElements.vue
@@ -42,11 +42,11 @@ const getLicenseDescription = (element: string) => {
 </script>
 
 <template>
-  <ul>
+  <ul class="flex flex-col gap-y-2 md:gap-y-4">
     <li
       v-for="element in elementNames"
       :key="element"
-      class="mb-2 flex items-center gap-3 text-sm md:mb-4 md:text-base"
+      class="flex items-center gap-x-3 text-sm md:text-base"
     >
       <VIcon
         view-box="0 0 30 30"

diff --git a/frontend/src/components/VMediaInfo/VMediaLicense.vue b/frontend/src/components/VMediaInfo/VMediaLicense.vue
@@ -36,7 +36,7 @@ const sendVisitLicensePage = () => {
 </script>
 
 <template>
-  <div class="media-attribution">
+  <div class="media-attribution mb-2 md:mb-0">
     <h3 class="description-bold md:heading-6 mb-4">
       {{ headerText }}
     </h3>