-
Notifications
You must be signed in to change notification settings - Fork 4
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
icon to userpage #225
icon to userpage #225
Conversation
支援課のアイコンは東工大のマークにとりあえずしておきます。公式HPのロゴのページでは余白がなくていい感じにならなかったので公式twitterのプロフィール画像からとっています。それとコンフリクトが怖かったのでissue-174もこっちにまとめてます。 |
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.
レビューしました
<v-btn | ||
dark | ||
icon | ||
:to="`/users/${user.name}`" | ||
> |
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.
なんかめっちゃインデントずれてる
<v-avatar :size="size"> | ||
<v-img :src="'https://q.trap.jp/api/1.0/public/icon/' + user.name" width="100%" alt=""/> | ||
<v-img :src="user.name!=='sienka'?'https://q.trap.jp/api/1.0/public/icon/' + user.name:'https://pbs.twimg.com/profile_images/877073041351622657/n31PKuuM.jpg'" width="100%" alt=""/> |
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.
東工大のアイコンは直リンクじゃなくてこっちで保持しようか
アイコンダウンロードしてclient/public/img/~~.jpg
に保存してここから呼ぶようにしてください
client/src/components/UserPage.vue
Outdated
@@ -8,7 +8,7 @@ | |||
</div> | |||
<div> | |||
<v-avatar size="200"> | |||
<img :src="`https://q.trap.jp/api/1.0/public/icon/${$route.params.name}`" /> | |||
<img :src="$route.params.name!=='sienka'?`https://q.trap.jp/api/1.0/public/icon/${$route.params.name}`:'https://pbs.twimg.com/profile_images/877073041351622657/n31PKuuM.jpg'" /> |
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.
支援課かどうかの判定のロジックはcomputed
に書いてほしいです(伝わるかな)
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.
遅くなりましたがコメント返しました
client/src/components/UserPage.vue
Outdated
} | ||
}, | ||
watch: { | ||
'$route' (to, from) { | ||
this.mount() | ||
} | ||
}, | ||
computed: { | ||
checkSienka () { | ||
return this.$route.params.name !== 'sienka' |
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.
URLまでここで返して大丈夫です
computedに移す目的としては上のになるべくjavascriptの構文を書かないという目的があるので(ほかのとこめっちゃ書いてるけど)
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.
コメントしました(あとちょっと!)
client/src/components/UserPage.vue
Outdated
} | ||
}, | ||
watch: { | ||
'$route' (to, from) { | ||
this.mount() | ||
} | ||
}, | ||
computed: { | ||
navigateImagePath () { | ||
return this.$route.params.name !== 'sienka' ? `https://q.trap.jp/api/1.0/public/icon/${this.$route.params.name}` : this.sienka |
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.
sienkaのiconPathが変わることはないからthis.sienka
ってせずに'./../img/sienka-icon.jpg'
でいいかも
}, | ||
computed: { | ||
navigateImagePath () { | ||
return this.user.name !== 'sienka' ? `https://q.trap.jp/api/1.0/public/icon/${this.user.name}` : this.sienka |
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.
ここも同じ
dataの行が減ってスマートになるね
@series2 修正お願いします! |
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.
LGTM:+1:
コンフリクトはたぶん大丈夫だと思います
あっているかわかりませんが、動いたのでこれでいきます