Skip to content

Fix/ replace hamburger menu with dialog element #379

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 17, 2024
Merged

Fix/ replace hamburger menu with dialog element #379

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d6fc734
fix/ replace hamburger menu with dialog element
totocalcio Oct 13, 2024
e6aedfd
Refactor GlobalHeader.vue styles
totocalcio Oct 13, 2024
0e6b766
fix: element that use autofocus
totocalcio Oct 13, 2024
9324547
Refactor GlobalHeader.vue styles: Update translate values for smooth …
totocalcio Oct 13, 2024
5c2e8d6
- remove aria-label from dialog element.
totocalcio Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 44 additions & 16 deletions apps/web/app/components/GlobalHeader.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const onSwitchLocale = () => {
const { width } = useWindowSize()
const { orientation } = useScreenOrientation()
const shouldShowSpHeader = ref()
const dialogRef = ref<HTMLDialogElement>()
onMounted(() => {
shouldShowSpHeader.value = width.value <= 1200
})
Expand All @@ -27,6 +28,11 @@ watch([width, orientation], () => {
const showMenu = ref(false)

const toggleMenu = () => {
if (showMenu.value) {
dialogRef.value!.close()
} else {
dialogRef.value!.show()
}
showMenu.value = !showMenu.value
}

Expand All @@ -48,26 +54,36 @@ const getAnchorPath = computed(
class="navigation-mobile-toggle"
name="menu"
:class="{ 'isOpened': showMenu }"
:aria-expanded="showMenu"
aria-controls="navigation-mobile-menu-trigger"
:aria-label="showMenu ? 'メニューを閉じる' : 'メニューを開く'"
@click="toggleMenu"
>
<span /><span /><span />
</button>
<!-- <VFIcon name="menu" color="vue-blue" can-hover @click="toggleMenu" /> -->
</div>
<!-- hamburger-menu -->
<Transition name="slide-down">
<div v-show="showMenu" class="navigation-mobile-menu">
<div>
<ul>
<li v-for="link in navLinks" :key="link.anchor">
<nuxt-link :to="getAnchorPath(link.anchor)" @click="toggleMenu">
<VFTypography variant="heading/200" color="vue-blue">{{ link.text }}</VFTypography>
</nuxt-link>
</li>
</ul>
</div>
</div>
</Transition>
<dialog
id="navigation-mobile-menu-trigger"
ref="dialogRef"
aria-label="ハンバーガーメニュー"
Copy link
Contributor

Choose a reason for hiding this comment

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

aria-label はなくて良いと思います。
以下に詳細記載しました。

#379 (comment)

Copy link
Contributor Author

@totocalcio totocalcio Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@KannoStanfoot @jiyuujin
レビューとTipsの共有ありがとうございました。
非常に参考になりました。今後ぜひ活用させて頂きます。

今回はaria-labelの削除のみ対応致しましたので、再度レビューお願い致します。

またNetlifyのデプロイがブロックされており、ブラウザ確認ができておりません。
一部CSSを修正したため、クロスブラウザで確認したいので、
プレビュー環境へ反映後チェックします。

class="navigation-mobile-menu"
>
<ul>
<li v-for="(link, index) in navLinks" :key="link.anchor">
<!-- eslint-disable vuejs-accessibility/no-autofocus -->
<nuxt-link
:to="getAnchorPath(link.anchor)"
:autofocus="index === 0 && true"
@click="toggleMenu"
>
<!-- eslint-enable vuejs-accessibility/no-autofocus -->
<VFTypography variant="heading/200" color="vue-blue">{{ link.text }}</VFTypography>
</nuxt-link>
</li>
</ul>
</dialog>
</VFSpHeader>
<VFHeader v-else>
<div class="navigation-pc">
Expand Down Expand Up @@ -95,6 +111,7 @@ const getAnchorPath = computed(
align-items: center;
gap: calc(var(--unit) * 2);
margin-right: 27px;
z-index: 10;
}

.navigation-mobile-menu {
Expand All @@ -104,9 +121,20 @@ const getAnchorPath = computed(
width: 100vw;
text-align: center;
background-color: var(--color-white);

& > div {
padding: calc(var(--unit) * 5) 0;
padding: calc(var(--unit) * 5) 0;
border: 0;
transition:
translate 0.6s cubic-bezier(0.4, 0, 0.2, 1),
display 0.6s cubic-bezier(0.4, 0, 0.2, 1) allow-discrete;
translate: 0 -100dvh;
z-index: 1;

&[open] {
translate: 0 0;

@starting-style {
translate: 0 -100dvh;
}
}

ul {
Expand Down
11 changes: 11 additions & 0 deletions packages/ui/components/common/SpHeader.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,20 @@ header {
display: flex;
align-items: center;
justify-content: space-between;
&::before {
content: '';
position: absolute;
top: 0;
left: 0;
width: 100%;
height: 100%;
background-color: var(--color-white);
z-index: 5;
}
}
.link {
line-height: 0;
z-index: 10;
}
.logo {
height: 32px;
Expand Down
Loading