コンテキストメニューのVue化 #1374

thiramisu · 2023-07-06T14:25:19Z

内容

コンテキストメニューの実装を Electron から Vue + Quasar に変更します。

設計の概要

既存のcomponents/MenuItem.vueをコンポーネントとして使用し、<q-menu touch-position context-menu><q-list dense>で囲いました。
コピーなどのアクションはpure javascript寄りでゴリゴリ書きました。
AudioCell.vueの// コピペしたときに句点と改行で区切る(175~213行目)のあたりは貼り付け処理を書きやすいようにリファクタリングをしただけです。

PR作成前からの不具合

その1: 以前の選択範囲が残る

詳細: #1364 (comment)
範囲選択状態からフォーカスを外し、そこからコンテキストメニューを開くと、なぜか以前の選択範囲が残ったままになる不具合です。以前の選択範囲を復元するのはinput.focus()時のデフォルトの動作のようです。調査に時間がかかりそうです。

その2: 特定条件下で貼り付け時にエラーが出る

空セルに実テキストが一行で末尾が\nの繰り返しで終わるテキスト(追記: 精確には改行を含むがAudioCellの数は増えないようなテキスト、でした)をCtrl+vで貼り付けようとすると開発者ツール上でエラーが出る不具合です。その動作を流用したためコンテキストメニューを使った貼り付けでも同じエラーが出ます。
フォーカスに失敗してるだけで致命的ではなさそうなので、直せたら別PRとして送ろうと思います。

スクリーンショット・動画など

その他

将来的な話:

範囲選択せずに[コピー]や[切り取り]した時にAudioCell全体のコピペができるようにするのもアリかなと思いました。
clipboardAPIを使うとBlobデータ(つまりjsonとか)もコピペできるはずですが、2つのVOICEVOX間でデータをコピペ、とかすることはなさそうですね…

新たに発生する不具合

[追記: 解消の目途が立ちました]
コンテキストメニューを開いている間、テキスト選択範囲 (青の反転) などが表示されなくなります。
~~原因不明で、現在調査中です。~~
詳細: #1364 (comment)

報告後に気付いた調査の抜け:

Context Menu example を使った調査では<div>内にテキストを追加して試したのですが、<input>内の選択範囲は通常の選択範囲とは別の実装で操作する必要があるのを考慮すると、<q-input>内では別の結果になる可能性があります。

まだ試してないこと:

上述の調査の抜け
ブラウザ版を使ってブラウザでの挙動を確認する
Quasarのバージョンアップ

[追記: ここまで解消の目途が立ちました]

Hiroshiba · 2023-07-06T16:31:16Z

src/components/ContextMenu.vue

@@ -0,0 +1,32 @@
+<template>
+  <q-menu touch-position context-menu>


ここに no-focus をつけると行けそうでした！
フォーカスが外れて（blurされて）選択状態が解除され、選択範囲のレンダリングが消えるのが原因かなと思いました。

ちなみにgoogle documentとかは右クリックしても選択範囲消えないんですよね。
なにかinput要素に対して特別なことをやってるのかなと思って調べてみたら、canvasでした 😇
↓ google documentで右クリックしている時の例

アクセシビリティに詳しそうな @yncat さんにお聞きしたいことが 🙇‍♂️
no-focusをつけると右クリックのメニューにフォーカスされないので、キーボードのみの方は右クリックメニューを実行するのが大変になりそうです。
これって結構不便だったりされますか･･･？
なんとなく、そもそも右クリックメニューはほぼ使わず、ショートカットキーで操作される方が多いのかなと想像しています。
一般的な右クリックとアクセシビリティのことをご存知でしたらご教示いただきたく･･･！！

正解がこんな足元に転がっているとは思っていませんでした…難しく考えすぎていました。
代わりに原因究明＆解決までしていただき、ありがとうございます！🙇‍♂️🙇‍♂️🙇‍♂️

フォーカスされないことで一部実装が変わりそうなので、その辺はまた考えます。
とりあえずno-focusはつけておきました！

google document

canvas消したら見た目からは文字とか選択範囲とか消えたんですが、見えない文字がコピペできる…？！一体どうなってるんでしょう…

まだ試してないですが、たぶんno-focus付けてしまうとキーボードでコンテキストメニュー使えなくなる気がする...
要動作検証

yncat · 2023-07-08T03:22:02Z

アクセシビリティ周りに関して、見解を共有します。

まずは、正直な話をします。
今のmain branchの状態では、最終的にOS標準のメニューとして描画されるようで、アクセシビリティが高いです。
これを vue で独自実装すると、全体的にスクリーンリーダーでの操作性はかなり低下します。具体的には、OS標準で使えるメニューのキーボード操作がほとんど利用できなくなり、サブメニューのハンドリングもやりにくくなります。

ただ、それをやらないでくださいというのは本末転倒だと思うので、前向きな方向で考えると。

最終的には、実際に動作を確認してみて、操作できるかできないか、ということになると思います。

このPRをローカルに持って来て実行しようとしたら、 error? warning? で実行できませんでした。この部分の修正いただくことってしゅっと可能でしょうか？

もしかしたら warning で、ウィンドウを消せば使えるかもしれないのですが、 click outside (ダイアログの外側をクリック) の操作が画面見ないとできないので、自分ではこの画面を dismiss できず先に進めない状況です。

[plugin:vite:vue] Duplicate attribute.
D:/git/voicevox/src/components/AudioCell.vue:42:7
40 |        @keydown.prevent.down.exact="moveDownCell"
41 |        @keydown.prevent.enter.exact="pushAudioTextIfNeeded()"
42 |        :aria-label="`${textLineNumberIndex}行目`"
   |         ^
43 |      >
44 |        <template #error>
    at createCompilerError (D:\git\voicevox\node_modules\@vue\compiler-core\dist\compiler-core.cjs.js:19:19)
    at emitError (D:\git\voicevox\node_modules\@vue\compiler-core\dist\compiler-core.cjs.js:1613:29)
    at parseAttribute (D:\git\voicevox\node_modules\@vue\compiler-core\dist\compiler-core.cjs.js:1354:9)
    at parseAttributes (D:\git\voicevox\node_modules\@vue\compiler-core\dist\compiler-core.cjs.js:1330:22)
    at parseTag (D:\git\voicevox\node_modules\@vue\compiler-core\dist\compiler-core.cjs.js:1199:17)
    at parseElement (D:\git\voicevox\node_modules\@vue\compiler-core\dist\compiler-core.cjs.js:1128:21)
    at parseChildren (D:\git\voicevox\node_modules\@vue\compiler-core\dist\compiler-core.cjs.js:943:28)
    at parseElement (D:\git\voicevox\node_modules\@vue\compiler-core\dist\compiler-core.cjs.js:1144:22)
    at parseChildren (D:\git\voicevox\node_modules\@vue\compiler-core\dist\compiler-core.cjs.js:943:28)
    at parseElement (D:\git\voicevox\node_modules\@vue\compiler-core\dist\compiler-core.cjs.js:1144:22
Click outside or fix the code to dismiss.
You can also disable this overlay by setting server.hmr.overlay to false in vite.config.js.

yncat · 2023-07-08T03:29:20Z

src/components/AudioCell.vue

      @paste="pasteOnAudioCell"
      @focus="setActiveAudioKey()"
      @keydown.prevent.up.exact="moveUpCell"
      @keydown.prevent.down.exact="moveDownCell"
-      @mouseup.right="onRightClickTextField"
+      @keydown.prevent.enter.exact="pushAudioTextIfNeeded()"
+      :aria-label="`${textLineNumberIndex}行目`"


なんか私が入れたやつが怒られてますね

Attribute ":aria-label" should go before "@keydown.prevent.enter.exact"

ほんとに順番入れ替えれば治る問題なんだろうかw

いったんこちらに関してだけコメントをば 🙇
これはエラーというよりワーニングで、並び順は順不同で複数人で書いてるとごちゃごちゃになっちゃうので、順番を作ってそれを守りましょうという感じの意図のメッセージです！

thiramisu · 2023-07-08T08:33:54Z

なるほど、勉強になります…

コンフリクトの解消についてよく分かってないのに適当にボタンポチポチしたせいで、2箇所エラー吐いてました！多分直りました。よろしくお願いします。

yncat · 2023-07-08T12:47:14Z

やっぱり、no-focusがついていることによって、スクリーンリーダーでコンテキストメニューに触ることが絶対に不可能になってしまいました。
フォーカスされないので、inputの中にずっといるように見えて、コンテキストメニューが開いたことすら認識できないです。

thiramisu · 2023-07-08T13:01:57Z

確認ありがとうございます！
windowsの標準録画機能にも映るようになってたり、矢印キーとかに反応しなくなってたりで、そんな気はしていました。

やっぱりelectronで実装する
no-focusを設定から変えられるようにする
何かの方法で選択範囲を表示する

選択肢はこんな感じで、3番目を最初に検討すべきでしょうか？
まあ原因は分かったので対処も以前よりしやすいかなと思ってます！

google documentはしっかり選択範囲を表示しつつTabキーとかで操作できてたので、謎のcanvasとかはそこら辺も考慮しての実装なのかもですね。

Hiroshiba · 2023-07-08T16:40:11Z

@yncat さんありがとうございます！！
no-focus、一番最初にfocusが移動することを抑制するだけかなと思ってたのですが、フォーカスすることがそもそもできなくなるんですね･････。

これは流石にだいぶ課題だなと感じました。

@thiramisu ３つの迂回策、同意見です。
３番めがやはりいちばん良いかなと思います。
quasarの中の、更にinputの中身をいじることになりそうなので、覚悟が必要そうですが･･･。

google doctumentほんとにすごいですね。。。
まあでもリッチなテキストエディタを作っている人は同じ課題に当たっているはずなので、その辺りの知見を活用できるかも？

そういえばニコ動も右クリックコンテキストメニューがあったので見てみたところ、クリックしたコメントがコンテキストメニューの一番上に表示される形になっていました！

長い文字列だった場合どうすればいいんだという問題はありますが、少なくとも表示範囲が消えるだけよりマシかな～と。
（一旦この妥協案もかなりありかなと思いました！）

thiramisu · 2023-07-22T07:23:19Z

選択範囲が残る挙動の変更、アプリ全体に適用されるようにしてみました。
ちなみにonBlur(onFocusout)での実装にしない理由は2点あります。

理由1.
以下だと正常に選択範囲を空にできるんですが、
https://codepen.io/thiramisu/pen/poQZbdr
実際にEditorHome.vueに以下を書くと、

  const on =
    //*/ この行先頭の/の付け外しで切り替え
    "focusout"; // eslint-disable-next-line prettier/prettier
    /*/
    "focusin"; // eslint-disable-next-line prettier/prettier
    //*/
  document.addEventListener(
    on,
    (event: FocusEvent) => {
      if (event.target instanceof HTMLInputElement) {
        event.target.selectionEnd = event.target.selectionStart;
        console.log(
          "to empty: ",
          event.target.selectionStart,
          ",",
          event.target.selectionEnd
        );
        console.log(event);
      }
    },
    { capture: true }
  );

発火自体はしていて、代入もされているんですが、なぜか効果がありません…
(念のため補足しておくと、ネイティブなエレメントに値を代入した場合、その時点で型チェックみたいなのが走るので、あえて不正な値を代入することで代入が実行されているかをある程度判断できます。例えば

        event.target.selectionEnd = -999;

とすると正常な値に補正されているのが分かると思います。)

理由2.
no-focusがついていないメニューを開く時はfocusEvent.relatedTarget === nullになるため、イベントがコンテキストメニュー由来かの判別が面倒になります。
閉じるときはno-focusありでもなしでもrelatedTargetが存在するので、判別が楽です。
参考: FocusEvent.relatedTarget

ちなみに、willDispatchFocusOrBlurも理由2により消せそうで消せません…回避のために開く際に手動でダミーエレメントをフォーカスすると、閉じる際に再フォーカスが必要になり、逆に処理が複雑になったりします。

Hiroshiba · 2023-07-22T12:13:29Z

本でいうところの節の境目を空行1行、章の境目を空行2行で表すみたいな使い方してたので、空行維持の方が使いやすかったです。

なるほどです、よくわかりました！！
ただ、一長一短あるときに仕様変更すると、元の方が都合が良かった人の「なぜ変えてしまったのか」という反応に応えづらかったりします。
プルリクエストの内容とずれた仕様変更になりますので、一旦元のままにして頂けますと幸いです。申し訳ない🙇

一旦issue化してみました。
#1417

mousetrapと干渉して

なるほどです、今試してみたらたしかに問題が発生していそうでした！（ワークアラウンド適用後のコードで試していたかもしれません）
ボタンの押下を代替するキーコマンドはエンター以外にスペースなどもあり、このワークアラウンドでは不十分かもしれません。
可能であればここのコメントのようにmousetrap側を一旦無効にするのが良いのかなとか思いました。

一旦ワークアラウンドなしでマージし、その後に修正を多角的に検討して試みるのはどうでしょう？
（重要度がある程度高い問題だという意識はありますが、このプルリクエストがいつまでもマージされないということになりそうなのでそろそろマージに向かいたいなと･･･！）

選択範囲が残る挙動の変更、アプリ全体に適用されるようにしてみました。

あーー全要素への適用は副作用が怖いので慎重になりたいかもしれません。
他の方が別のことを試してなにか妙な挙動があったとき、流石にここに辿り着けないので･･･････。
focusが当たり直すことがよくあり、選択範囲が残ってると不便なのは今のところAudioCellだけなので、影響範囲をここに絞っておくのはどうでしょう 🙇

Hiroshiba · 2023-07-22T12:14:56Z

コード変更ありがとうございます！
ちょっと変更が重なっていて、一度レビューしたコードを何度もレビューするため若干効率が悪く感じ始めました。
一度マージを目指せれると幸いです！！

thiramisu · 2023-07-22T15:54:18Z

とりあえずマージを目指す、了解です！加減が分かっておらず、すいません。止めていただけて助かります。

一旦issue化してみました。

ありがとうございます。こちらは一応の確認をしたまでで、挙動はほぼ以前のままなので、マージ的な面では大丈夫だと思います。

エンター以外にスペースなどもあり

以下の実装を参考にして、ある程度妥当な実装にはできました。
https://craig.is/killing/mice#api.stopCallback
https://github.com/ccampbell/mousetrap/blob/master/plugins/pause/mousetrap-pause.js
同じ<q-item>だったのでついでに#1416 も直ったみたいです。
それはそれとしてスペースキーはまだ効かないのですが、Quasar 公式の例のサンプルでも効かないので、仕様に近いのかなと思います。
エンターはこの実装で、スペースのほうはひとまず諦めて良いと思うのですが、どうでしょうか？
もしこの実装自体が妥当でなさそうであれば、マージを優先し、エンターキーも諦めようかと思います。

全要素への適用は副作用が怖い

確かにそうですね、AudioCellのみの版に戻しておきました。

一応全てResolvedにはなっているので、他にやり残していることなどもあれば教えていただけると助かります。

Hiroshiba · 2023-07-22T17:39:30Z

以下の実装を参考にして、ある程度妥当な実装にはできました。

あーーーーなるほどです！！！！
なぜこのメニューでだけ問題が発生していてのか疑問だったのですが、たぶんq-itemが<div>でここにfocusが当たっていたからeventの伝播が止まらない、とかですかね！！
stopCallbackを上書きするの、良い手法に思いました！

Hiroshiba

ほぼほぼLGTMです！！！

１点アニメーションをなくす提案をしていて、これがもしかしたら結構変更を必要とするかもしれません。
その場合は申し訳ないです。。。

src/components/AudioCell.vue

src/components/ContextMenu.vue

src/views/EditorHome.vue

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>

into context-menu-vue

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>

into context-menu-vue

イベント名だけそのまま

Hiroshiba · 2023-07-22T22:55:14Z

LGTM！！！！！！！！！！！！！！！

いや～～～～～～～～～かなり手強かったですね！！！！！！！
「操作を作る」点でとても良い勉強になりました。お疲れ様でした！！！

（@yncat さんもありがとうございました！）

これでもともとのモチベーションであった「見た目上のテキストだけを編集」する機能の追加
#1353のスキルツリーが解禁された形でしょうか。

じつは右クリックメニューに関してもう１つあったりします。こちらはこちらでどうやってダイアログに文字を渡すのかという課題がありそうですが、もしご興味あればぜひ･･･！！！！

辞書の単語追加・編集UIの単語・読み入力欄で右クリックメニューを使えるようにする #747

thiramisu added 12 commits July 4, 2023 06:31

見た目上だけ整えた

88c4ada

denseが抜けてたのを修正

7a48ed3

各アイテム選択時の動作を追加

6c853c2

コメントアウト内容修正

3b81808

読み更新処理が発火しないバグを修正

32ab2dc

コメントアウト修正

92e32eb

未使用になったelectron/contextMenu.ts関連の記述を削除

742e258

関数名の見直しと共通化

d928868

pasteOnAudioCell関数の一部を別関数に切り分け

c7a54e3

コメントアウトのミスの修正

a35cc73

複数行ペースト処理周りのリファクタリング

5962c2c

コンテキストメニューにも複数行貼り付けを追加

59701b8

Hiroshiba reviewed Jul 6, 2023

View reviewed changes

thiramisu added 4 commits July 7, 2023 02:20

コメントアウト消し忘れ

5bff4f3

no-focusを追加

2f0e943

リファクタリング

b46cb46

Merge branch 'main' into context-menu-vue

a027074

yncat reviewed Jul 8, 2023

View reviewed changes

thiramisu added 2 commits July 8, 2023 17:19

コンフリクトの解消にし失敗してた箇所の修正

e8e9645

コンフリクトの解消に失敗していたのを修正

908074c

thiramisu added 2 commits July 8, 2023 18:23

コンテキストメニュー開閉時に読みが更新されてしまうバグを修正

390fd6d

変数名を見直し

2f1bb26

thiramisu added 3 commits July 9, 2023 04:36

コンテキストメニューにヘッダー追加

3eb2d15

省略時の見た目を少し改善

4052464

未選択時の挙動を追加

2c7c04e

`willDispatchFocusOrBlurをAudioCell.vueに移動

65a3f8a

Hiroshiba mentioned this pull request Jul 22, 2023

複数行貼り付け時・テキスト読み込み時に改行を無視するかどうかを設定できるようにする #1417

Closed

thiramisu added 3 commits July 23, 2023 00:00

Mousetrap回避策の見直し

0ed134b

選択範囲が残らないようにするコードをAudioCell内に戻す

973d7b8

戻したのに伴って不要になったコードを削除

645c10a

コメントアウトのコピペミスを修正

7b1b72a

Hiroshiba reviewed Jul 22, 2023

View reviewed changes

thiramisu and others added 9 commits July 23, 2023 05:33

変数宣言をなるべく使用直前に

8d5afc8

hideすることを明示

9e27cc9

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>

Merge branch 'context-menu-vue' of https://github.com/thiramisu/voicevox

e46dba2

into context-menu-vue

不要な<style>を削除

c7deec7

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>

コメントアウトを修正

0b1fa27

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>

アニメーションをなくす

5775cc8

Merge branch 'context-menu-vue' of https://github.com/thiramisu/voicevox

4d827fa

into context-menu-vue

getMenuItemButtonをreadyForContextMenu内に

8f60536

contextmenuかcontextMenuかの表記揺れを修正

e762a04

イベント名だけそのまま

thiramisu requested a review from Hiroshiba July 22, 2023 21:15

コンフリクト解消

cb11a25

Hiroshiba merged commit b8e08ed into VOICEVOX:main Jul 22, 2023

This was referenced Jul 3, 2023

「見た目上のテキストだけを編集」する機能の追加 #1353

Closed

エンジン起動中にキー入力などをするとその後キーが効かない場面がある #1416

Closed

thiramisu deleted the context-menu-vue branch July 22, 2023 23:59

thiramisu added a commit to thiramisu/voicevox that referenced this pull request Jul 26, 2023

VOICEVOX#1374 でも同じ実装をした覚えがあったのでついでに修正

94120eb

Hiroshiba mentioned this pull request Oct 29, 2023

辞書の単語追加・編集UIの単語・読み入力欄で右クリックメニューを使えるようにする #747

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

コンテキストメニューのVue化 #1374

コンテキストメニューのVue化 #1374

thiramisu commented Jul 6, 2023 •

edited

Loading

Hiroshiba Jul 6, 2023 •

edited

Loading

thiramisu Jul 6, 2023 •

edited

Loading

yncat Jul 8, 2023

yncat commented Jul 8, 2023

yncat Jul 8, 2023

Hiroshiba Jul 8, 2023 •

edited

Loading

thiramisu commented Jul 8, 2023

yncat commented Jul 8, 2023 •

edited

Loading

thiramisu commented Jul 8, 2023 •

edited

Loading

Hiroshiba commented Jul 8, 2023 •

edited

Loading

thiramisu commented Jul 22, 2023 •

edited

Loading

Hiroshiba commented Jul 22, 2023

Hiroshiba commented Jul 22, 2023

thiramisu commented Jul 22, 2023 •

edited

Loading

Hiroshiba commented Jul 22, 2023 •

edited

Loading

Hiroshiba left a comment

Hiroshiba commented Jul 22, 2023

		@@ -0,0 +1,32 @@
		<template>
		<q-menu touch-position context-menu>

コンテキストメニューのVue化 #1374

コンテキストメニューのVue化 #1374

Conversation

thiramisu commented Jul 6, 2023 • edited Loading

内容

設計の概要

PR作成前からの不具合

その1: 以前の選択範囲が残る

その2: 特定条件下で貼り付け時にエラーが出る

関連 Issue / PR

スクリーンショット・動画など

その他

新たに発生する不具合

Hiroshiba Jul 6, 2023 • edited Loading

Choose a reason for hiding this comment

thiramisu Jul 6, 2023 • edited Loading

Choose a reason for hiding this comment

yncat Jul 8, 2023

Choose a reason for hiding this comment

yncat commented Jul 8, 2023

yncat Jul 8, 2023

Choose a reason for hiding this comment

Hiroshiba Jul 8, 2023 • edited Loading

Choose a reason for hiding this comment

thiramisu commented Jul 8, 2023

yncat commented Jul 8, 2023 • edited Loading

thiramisu commented Jul 8, 2023 • edited Loading

Hiroshiba commented Jul 8, 2023 • edited Loading

thiramisu commented Jul 22, 2023 • edited Loading

Hiroshiba commented Jul 22, 2023

Hiroshiba commented Jul 22, 2023

thiramisu commented Jul 22, 2023 • edited Loading

Hiroshiba commented Jul 22, 2023 • edited Loading

Hiroshiba left a comment

Choose a reason for hiding this comment

Hiroshiba commented Jul 22, 2023

thiramisu commented Jul 6, 2023 •

edited

Loading

Hiroshiba Jul 6, 2023 •

edited

Loading

thiramisu Jul 6, 2023 •

edited

Loading

Hiroshiba Jul 8, 2023 •

edited

Loading

yncat commented Jul 8, 2023 •

edited

Loading

thiramisu commented Jul 8, 2023 •

edited

Loading

Hiroshiba commented Jul 8, 2023 •

edited

Loading

thiramisu commented Jul 22, 2023 •

edited

Loading

thiramisu commented Jul 22, 2023 •

edited

Loading

Hiroshiba commented Jul 22, 2023 •

edited

Loading