Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Typescriptify VModal components #1926

Merged
merged 3 commits into from
Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/assets/icons/openverse-logo-text.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
24 changes: 15 additions & 9 deletions src/components/VHeaderOld/VLogoButtonOld.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,19 @@
:status="isFetching ? 'loading' : 'idle'"
:auto-resize="autoResizeLogo"
/>
<OpenverseLogoText
<svg
v-show="!isHeaderScrolled"
class="-ml-1 mt-1 me-3"
:class="{
'hidden sm:block md:hidden': isSearchRoute,
}"
:aria-hidden="true"
class="-ml-1 mt-1 flex flex-shrink-0 flex-grow-0 me-3"
:class="{ 'hidden sm:block md:hidden': isSearchRoute }"
xmlns="http://www.w3.org/2000/svg"
width="95"
height="15"
/>
viewBox="0 0 95 15"
aria-hidden="true"
focusable="false"
>
<use :href="`${OpenverseLogoText}#logo`" />
</svg>
</VButton>
</template>
<script>
Expand All @@ -29,11 +32,11 @@ import { defineComponent } from '@nuxtjs/composition-api'
import VLogoLoader from '~/components/VLogoLoader/VLogoLoader.vue'
import VButton from '~/components/VButton.vue'

import OpenverseLogoText from '~/assets/icons/openverse-logo-text.svg?inline'
import OpenverseLogoText from '~/assets/icons/openverse-logo-text.svg'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember if there was a specific reason we are getting rid of ?inline. But this is leading to the SVG tag being duplicated in both the SVG file and in the HTML.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote about the reasons in this issue: https://github.com/WordPress/openverse-frontend/issues/1715.
I haven't noticed the duplication, I'll look into it, thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the SVG tag duplication, @dhruvkb:
Screen Shot 2022-10-27 at 9 23 40 PM


export default defineComponent({
name: 'VLogoButtonOld',
components: { OpenverseLogoText, VLogoLoader, VButton },
components: { VLogoLoader, VButton },
props: {
isFetching: {
type: Boolean,
Expand All @@ -52,5 +55,8 @@ export default defineComponent({
default: true,
},
},
setup() {
return { OpenverseLogoText }
},
})
</script>
39 changes: 16 additions & 23 deletions src/components/VModal/VModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,22 @@
</div>
</template>

<script>
<script lang="ts">
import {
defineComponent,
ref,
watch,
reactive,
computed,
toRef,
PropType,
Ref,
} from '@nuxtjs/composition-api'

import { useBodyScrollLock } from '~/composables/use-body-scroll-lock'

import type { ModalColorMode, ModalVariant } from '~/types/modal'

import VModalContent from '~/components/VModal/VModalContent.vue'

export default defineComponent({
Expand Down Expand Up @@ -106,9 +110,7 @@ export default defineComponent({
* @default undefined
*/
initialFocusElement: {
type: /** @type {import('@nuxtjs/composition-api').PropType<HTMLElement>} */ (
process.server ? Object : HTMLElement
),
type: (process.server ? Object : HTMLElement) as PropType<HTMLElement>,
default: undefined,
},
/**
Expand All @@ -122,9 +124,7 @@ export default defineComponent({
* @default 'default'
*/
variant: {
type: /** @type {import('@nuxtjs/composition-api').PropType<'default' | 'full' | 'two-thirds'>} */ (
String
),
type: String as PropType<ModalVariant>,
default: 'default',
},
/**
Expand All @@ -135,9 +135,7 @@ export default defineComponent({
* @default 'light'
*/
mode: {
type: /** @type {import('@nuxtjs/composition-api').PropType<'dark' | 'light'>} */ (
String
),
type: String as PropType<ModalColorMode>,
default: 'light',
},
/**
Expand Down Expand Up @@ -167,29 +165,24 @@ export default defineComponent({
],
setup(props, { emit }) {
const visibleRef = toRef(props, 'visible')
const internalVisibleRef =
/** @type {import('@nuxtjs/composition-api').Ref<boolean>} */ (
ref(props.visible === undefined ? false : props.visible)
)
const nodeRef = ref()
const internalVisibleRef: Ref<boolean> = ref<boolean>(
typeof props.visible === 'undefined' ? false : props.visible
)
const nodeRef = ref<null | HTMLElement>(null)

/** @type {import('@nuxtjs/composition-api').Ref<HTMLElement | undefined>} */
const triggerContainerRef = ref()
const triggerContainerRef = ref<HTMLElement | null>(null)

const triggerA11yProps = reactive({
'aria-expanded': false,
'aria-haspopup': 'dialog',
})

const triggerRef = computed(
() =>
/** @type {HTMLElement | undefined} */ (
triggerContainerRef.value?.firstChild
)
() => triggerContainerRef.value?.firstChild as HTMLElement | undefined
)

watch(internalVisibleRef, (visible) => {
triggerA11yProps['aria-expanded'] = !!visible
triggerA11yProps['aria-expanded'] = visible
})

/**
Expand Down Expand Up @@ -223,7 +216,7 @@ export default defineComponent({
}

const onTriggerClick = () => {
if (internalVisibleRef.value === true) {
if (internalVisibleRef.value) {
close()
} else {
open()
Expand Down
39 changes: 19 additions & 20 deletions src/components/VModal/VModalContent.vue
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,22 @@
</VTeleport>
</template>

<script>
import { defineComponent, toRefs, ref, computed } from '@nuxtjs/composition-api'
<script lang="ts">
import {
defineComponent,
toRefs,
ref,
computed,
PropType,
} from '@nuxtjs/composition-api'
import { FocusTrap } from 'focus-trap-vue'
import { Portal as VTeleport } from 'portal-vue'

import { useDialogContent } from '~/composables/use-dialog-content'
import { warn } from '~/utils/console'

import type { ModalColorMode, ModalVariant } from '~/types/modal'

import VButton from '~/components/VButton.vue'
import VIcon from '~/components/VIcon/VIcon.vue'
import VLogoButtonOld from '~/components/VHeaderOld/VLogoButtonOld.vue'
Expand All @@ -93,9 +101,7 @@ export default defineComponent({
required: true,
},
hide: {
type: /** @type {import('@nuxtjs/composition-api').PropType<() => void>} */ (
Function
),
type: Function as PropType<() => void>,
required: true,
},
hideOnEsc: {
Expand All @@ -115,26 +121,19 @@ export default defineComponent({
default: true,
},
triggerElement: {
type: /** @type {import('@nuxtjs/composition-api').PropType<HTMLElement>} */ (
process.server ? Object : HTMLElement
),
type: (process.server ? Object : HTMLElement) as PropType<HTMLElement>,
default: null,
},
initialFocusElement: {
type: /** @type {import('@nuxtjs/composition-api').PropType<HTMLElement>} */ (
process.server ? Object : HTMLElement
),
required: false,
type: (process.server ? Object : HTMLElement) as PropType<HTMLElement>,
default: null,
},
variant: {
type: /** @type {import('@nuxtjs/composition-api').PropType<'default' | 'full' | 'two-thirds'>} */ (
String
),
type: String as PropType<ModalVariant>,
default: 'default',
},
mode: {
type: /** @type {import('@nuxtjs/composition-api').PropType<'dark' | 'light'>} */ (
String
),
type: String as PropType<ModalColorMode>,
default: 'light',
},
/**
Expand All @@ -152,11 +151,11 @@ export default defineComponent({
}

const propsRefs = toRefs(props)
const closeButton = ref()
const closeButton = ref<InstanceType<typeof VButton> | null>(null)
const initialFocusElement = computed(
() => props.initialFocusElement || closeButton.value?.$el
)
const dialogRef = ref()
const dialogRef = ref<HTMLElement | null>(null)
const { onKeyDown, onBlur } = useDialogContent({
dialogRef,
visibleRef: propsRefs.visible,
Expand Down
1 change: 0 additions & 1 deletion src/components/VModal/VSidebarTarget.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,5 @@ import { PortalTarget as VTeleportTarget } from 'portal-vue'
export default defineComponent({
name: 'VSidebarTarget',
components: { VTeleportTarget },
props: { element: { type: String, default: 'div' } },
})
</script>
2 changes: 2 additions & 0 deletions src/types/modal.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export type ModalVariant = 'default' | 'full' | 'two-thirds'
export type ModalColorMode = 'dark' | 'light'
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
"src/components/VLogoLoader/**.vue",
"src/components/VMediaInfo/**.vue",
"src/components/VMediaTag/**.vue",
"src/components/VModal/**.vue",
"src/components/VPopover/**.vue",
"src/components/VRecentSearches/**.vue",
"src/components/VSkeleton/**.vue",
Expand All @@ -85,7 +86,6 @@
"src/components/VHeader/VPageLinks.vue",
"src/components/VHeader/VSearchBar/**.vue",
"src/components/VImageDetails/VRelatedImages.vue",
"src/components/VModal/VInputModal.vue",
"src/pages/feedback.vue",
"src/pages/search-help.vue",

Expand Down