-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix/navigationbar #45
Conversation
.icon { | ||
cursor: pointer; | ||
color: $color-secondary; | ||
&:hover { | ||
opacity: 0.8; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iconってなんかdisplay: flex;
つけないと下にちょっと余白できちゃって高さが微妙な感じになるので毎回つけてるんですけど、Iconコンポーネント自体をいじった方がいいんですかね🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
確かに、そうした方が便利かもですね。
|
||
const isActive = computed(() => { | ||
if (props.path === '/') return currentRoute.path === props.path | ||
return currentRoute.path.startsWith(`${props.path}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここ前に気になってたので実装してもらえて:tasukaru::gozaimasu:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
すみません追加で🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
書き方ちょっとだけ気になったところ指摘したのですが、後は大丈夫そうです
感謝❗️🙌✨感謝❗️🙌✨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
だいたい良さそうです:+1:
細かい部分ばかりなんですが、気づいたことを書いておいたので確認していただけると助かります:pray:
import NavigationLinksItem from './NavigationLinksItem.vue' | ||
import { routes } from '/@/use/routeInfo' | ||
</script> | ||
|
||
<template> | ||
<ul> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ulの子要素は0個以上のli、script、templateらしいです。
今、navigation-links-item
がaタグで、直下にaタグが来てるのでliタグが来るように修正したほうがいいかも?です。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aタグは透過的コンテンツなのでul>a>liの構造でもul>liとみなせて問題ないはずです (もちろん逆でも問題はない)
https://developer.mozilla.org/ja/docs/Web/HTML/Element/a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
確かに大丈夫そうですね。ありがとうございます:pray:
import NavigationLinksItem from './NavigationLinksItem.vue' | ||
import { routes } from '/@/use/routeInfo' | ||
</script> | ||
|
||
<template> | ||
<ul> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aタグは透過的コンテンツなのでul>a>liの構造でもul>liとみなせて問題ないはずです (もちろん逆でも問題はない)
https://developer.mozilla.org/ja/docs/Web/HTML/Element/a
No description provided.