Skip to content
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

Jump to bottom

fix(frontend): MkPopupMenuがドロワーで子メニューの出現と同時にpopupをresolveさせるのをやめさせる #11441

Merged
merged 25 commits into from
Aug 9, 2023
Merged

fix(frontend): MkPopupMenuがドロワーで子メニューの出現と同時にpopupをresolveさせるのをやめさせる #11441

merged 25 commits into from
Aug 9, 2023

Conversation

tamaina
Copy link
Contributor

@tamaina tamaina commented Aug 1, 2023 •
edited
Loading

#11439 がモバイルで動かないのを修正

  • mouseenter周りのよくわからない挙動を回避する
  • asDrawerの時にMkPopupMenuでclosedが発行されるタイミングを調整

Resolve #10362
Resolve #11257
Resolve #10620

Additional info (optional)

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@github-actions github-actions bot added the packages/frontend Client side specific issue/PR label Aug 1, 2023
@codecov
Copy link

codecov bot commented Aug 1, 2023 •
edited
Loading

Codecov Report

Merging #11441 (fb924e6) into develop (b439067) will increase coverage by 0.00%.
Report is 4 commits behind head on develop.
The diff coverage is 92.94%.

@@           Coverage Diff            @@
##           develop   #11441   +/-   ##
========================================
  Coverage    78.61%   78.62%           
========================================
  Files          920      920           
  Lines        97388    97440   +52     
  Branches      7695     7694    -1     
========================================
+ Hits         76563    76608   +45     
- Misses       20825    20832    +7
Files Changed Coverage Δ
packages/frontend/src/scripts/get-note-menu.ts 5.11% <0.00%> (-0.03%) ⬇️
packages/frontend/src/scripts/get-user-menu.ts 5.55% <0.00%> (-0.04%) ⬇️
packages/frontend/src/components/MkMenu.vue 100.00% <100.00%> (ø)
packages/frontend/src/components/MkPopupMenu.vue 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@tamaina
Copy link
Contributor Author

tamaina commented Aug 1, 2023

あれ、治らない

@tamaina
Copy link
Contributor Author

tamaina commented Aug 1, 2023

しゅいろが言うように面倒なのでモーダルにするか

@tamaina
Copy link
Contributor Author

tamaina commented Aug 1, 2023

（なんでバグってるのかというと多分普通にchildrenCacheのせいなんだよな

@tamaina
Copy link
Contributor Author

tamaina commented Aug 1, 2023

（なんでバグってるのかというと多分普通にchildrenCacheのせいなんだよな

違う

@tamaina
Copy link
Contributor Author

tamaina commented Aug 1, 2023

MkModalがmousedownでclose()するのが原因

@tamaina
Copy link
Contributor Author

tamaina commented Aug 1, 2023

タップがbgに抜けている…？そういうもの？

@tamaina
Copy link
Contributor Author

tamaina commented Aug 1, 2023

（マウスイベントってz-indexガン無視で後ろに抜けるんだっけか）

@tamaina
Copy link
Contributor Author

tamaina commented Aug 1, 2023

TODO: stopImmediatePropagationを試してみる

@tamaina
Copy link
Contributor Author

tamaina commented Aug 1, 2023

mouseenterがあるとmousedownが呼ばれないの？

@tamaina
Copy link
Contributor Author

tamaina commented Aug 2, 2023

ていうかparent itemはタップしてもmousedownを発火しない（他の要素とは挙動が異なる…）

なぜ？

@tamaina
Copy link
Contributor Author

tamaina commented Aug 2, 2023

あっ、mousedownは発火しないわ

@tamaina
Copy link
Contributor Author

tamaina commented Aug 2, 2023

いいえmousedownは発火します

@tamaina
Copy link
Contributor Author

tamaina commented Aug 3, 2023

とにかくmouseenter/mouseoverがタッチと相性悪いっぽい

@tamaina tamaina marked this pull request as ready for review August 3, 2023 03:15
@tamaina
Copy link
Contributor Author

tamaina commented Aug 3, 2023

タッチかどうかでmouseenterとclickを使い分けるようにした

@github-actions github-actions bot requested a review from EbiseLutica August 3, 2023 03:16
@tamaina
Copy link
Contributor Author

tamaina commented Aug 3, 2023

リアクションが2回目以降できなくなった件

@tamaina tamaina marked this pull request as draft August 3, 2023 04:48
@tamaina
Copy link
Contributor Author

tamaina commented Aug 4, 2023

MkModalを改造する必要ないなこれ

@tamaina
Copy link
Contributor Author

tamaina commented Aug 4, 2023

MkPopupMenuだけで完結するようにできたけど、今度はポップアップメニューが2度表示されなくなった件

@futchitwo
Copy link
Contributor

タッチかどうかでmouseenterとclickを使い分けるようにした

close #10362 #11257 っぽい
あとタッチかじゃなくてドロワーかどうかで判定するようにしたら #10620 も解決しそう

@tamaina
Copy link
Contributor Author

tamaina commented Aug 4, 2023

あとタッチかじゃなくてドロワーかどうかで判定するようにしたら

やった

@tamaina
Copy link
Contributor Author

tamaina commented Aug 4, 2023

2度表示されなくなった

focus周りだったりするのかしら

@tamaina
Copy link
Contributor Author

tamaina commented Aug 4, 2023

あっ、MkModalがpointer-eventsをいじってたんだわ

@tamaina tamaina marked this pull request as ready for review August 4, 2023 14:46
@tamaina
Copy link
Contributor Author

tamaina commented Aug 4, 2023

動いた

@github-actions github-actions bot requested a review from syuilo August 4, 2023 14:47
syuilo
syuilo reviewed Aug 6, 2023
View reviewed changes
packages/frontend/src/components/MkMenu.vue Outdated
}
})();

if (!item.noCache) {
Copy link
Member

Choose a reason for hiding this comment

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

noCacheはどんな時に使うことを想定してる？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

表示ごとに変更されるようなものを想定していたけど要らないかも

（このPRでごちゃごちゃやっているときには使っていた

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noCacheは消した

@syuilo syuilo merged commit e6f3dd8 into develop Aug 9, 2023
@syuilo syuilo deleted the fix-menu branch August 9, 2023 00:08
@syuilo
Copy link
Member

syuilo commented Aug 9, 2023

👍

@anatawa12 anatawa12 mentioned this pull request Aug 16, 2023
Closed
anatawa12 pushed a commit to anatawa12/misskey that referenced this pull request Aug 16, 2023
@tamaina @anatawa12
fix(frontend): MkPopupMenuがドロワーで子メニューの出現と同時にpopupをresolveさせるのをやめさせる (m…
ec7aa63 
…isskey-dev#11441)

* fix(frontend): MkPopupMenuがドロワーで子メニューの出現と同時にpopupをresolveさせるのをやめさせる

* fix

* noCache

* ✌️

* fix

* ????

* a

* a

* ✌️

* fix emoji picker

* ?????

* close

* 1

* fix2

* ✌️

* fix

* ✌️

* ✌️

* ✌️

* preferClick

* ✌️

* fix lint

* a

* rm nocache
syuilo
syuilo reviewed Sep 23, 2023
View reviewed changes
packages/frontend/src/components/MkMenu.vue
@@ -39,11 +39,11 @@ SPDX-License-Identifier: AGPL-3.0-only
<MkSwitchButton :class="$style.switchButton" :checked="item.ref" :disabled="item.disabled" @toggle="switchItem(item)" />
<span :class="$style.switchText">{{ item.text }}</span>
</button>
<button v-else-if="item.type === 'parent'" role="menuitem" :tabindex="i" class="_button" :class="[$style.item, $style.parent, { [$style.childShowing]: childShowingItem === item }]" @mouseenter="showChildren(item, $event)">
<div v-else-if="item.type === 'parent'" role="menuitem" :tabindex="i" :class="[$style.item, $style.parent, { [$style.childShowing]: childShowingItem === item }]" @mouseenter="preferClick ? null : showChildren(item, $event)" @click="!preferClick ? null : showChildren(item, $event)">
Copy link
Member

Choose a reason for hiding this comment

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

button ではなく div に変えたのって何か意図がある？

@syuilo
Copy link
Member

syuilo commented Sep 23, 2023

なんか子メニューの表示位置がおかしくなる時があるわね

@syuilo
Copy link
Member

syuilo commented Sep 23, 2023

直った
76c4fed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@syuilo syuilo syuilo left review comments

@EbiseLutica EbiseLutica Awaiting requested review from EbiseLutica

Assignees
No one assigned
Labels
packages/frontend Client side specific issue/PR
Projects
None yet
Milestone
No milestone
3 participants
@tamaina @syuilo @futchitwo