-
Notifications
You must be signed in to change notification settings - Fork 42
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
リアクションをホバーしたときに拡大して表示 #4434
base: master
Are you sure you want to change the base?
リアクションをホバーしたときに拡大して表示 #4434
Conversation
Preview (prod) → https://4434-prod.traq-preview.trapti.tech/ |
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.
僕も a11y に詳しいわけではないので、どのように対応するのがいいかについてはっきりということはできないんですが、とりあえずここで示した部分について改めて調べて取り組んでみてほしいです…!
@@ -4,6 +4,8 @@ | |||
:title="tooltip" |
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.
title
属性を消去してaria-label
属性に切り替えました
Discordを参考にしてaria-label
の内容を定めたのでa11yの観点からも問題はないと思われます
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.
対応完了しました |
確認したんですが、今の状態でも 2 列に収まらない場合はそれ以降のスタンプを押した人の名前が表示されなくなってそうです (本番環境で確認しました) |
リアクションした人数が3人を超過した場合に「他n人」と圧縮表示するよう変更しました |
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-nya:
.scaleReaction { | ||
@include background-tertiary; | ||
display: flex; | ||
height: 3.5rem; |
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.
ここで高さが指定されていると思うんですが、これをしなくてもいいのかなと思いました
現状だと親要素の message-element
での overflow
や contain: content;
オプションが効いているので absolute で位置を指定した tooltip がメッセージの領域をはみ出したときに欠けてしまいます
多分 UN くんはこれを見てはみ出さないように高さを調整してくれたんだと思っているんですが、スタンプパレットのように teleport
タグとかを使いながら、メッセージ要素の子要素ではなくすことでその制約を考えなくて済むようになるのかな、と
const hoverTimeout = ref<ReturnType<typeof setTimeout> | null>(null) | ||
const isLongHovered = ref(false) | ||
|
||
watch(isHovered, beginHover => { |
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.
即時表示に戻さず、Discordに合わせることにしました。
@include background-tertiary; | ||
display: flex; | ||
height: 3.5rem; | ||
align-items: flex-start; |
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.
高さを可変にすることを考えると、これは上下中央揃えにしたほうが見栄えが良さそうな気がします
<div v-for="user in limitedUsers" :key="user.id" :class="$style.contents"> | ||
<stamp-detail-element-content | ||
:user-id="user.id" | ||
:count="user.count" | ||
:class="$style.content" | ||
/> | ||
<span | ||
v-if=" | ||
(!isLastUser(user) && !isSecondLastUser(user)) || | ||
isOverLimitSecondUser(user) | ||
" | ||
:class="$style.contents" | ||
> | ||
、 | ||
</span> | ||
<span | ||
v-if="isSecondLastUser(user) && !isOverLimitSecondUser(user)" | ||
:class="$style.contents" | ||
>と</span | ||
> | ||
<span v-if="isOverLimitUser(user)" :class="$style.contents" | ||
>と他{{ props.stamp.users.length - 3 }}人</span | ||
> | ||
<span v-if="isLastUser(user)" :class="$style.contents"> | ||
にリアクションされました | ||
</span> |
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-for
とかを使ってうまく表現してくれているんですが、 span 要素に表示する内容を直接 scripts
部分で生成してからここに持ってくるようにしてもいいのかなと思いました
詳細表示しているときならユーザー名でクリックできるようにするのもあって、クリック判定が別々になるように書かれていると思うんですけど、今回の場合はクリックされることを考えなくてよさそうですし
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4434 +/- ##
==========================================
- Coverage 86.41% 9.23% -77.18%
==========================================
Files 66 652 +586
Lines 4747 28105 +23358
Branches 503 521 +18
==========================================
- Hits 4102 2595 -1507
- Misses 639 25504 +24865
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
リアクションをホバーしたときに拡大して表示して欲しい #4433