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.

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

マルチデバイス同期機能追加 #205

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

walnuts1018
Copy link
Contributor

@walnuts1018 walnuts1018 commented Apr 11, 2023

マルチデバイス同期機能追加

Description

chrome.storage.localchrome.storage.syncに変更することで、複数デバイス間で課題の完了状態・設定を共有できるようにしました。

ただし、そのままでは1オブジェクトのサイズ制限に引っかかってしまうため、[hostname]-[key] の形でAssignmentsやQuiz, Settingsなどを別々に保存するように変更しました。

storageに対して変更が加わっていて問題が発生する可能性もあるため、sync機能はデフォルトでオフにし、「開発者設定」として有効化できるようにしています。
2023-04-11_20h42_33

chromium版Microsoft Edgeについてはテストを行い、複数デバイス同期に成功しましたが、chrome, firefox, safariについてはテストできていません。(ストアからインストールしないと同期のテストができないため。Edgeについては"拡張機能のパック" 機能を用いてテストができた。)
しかし、chrome, firefoxでchrome.storage.syncに保存できていて、問題なく動いているということ自体は確認できました。

panda.mp4

変更点が多かったので不備があるかもしれません。ご確認よろしくお願いいたします。

Type of change

  • バグ修正
  • 新機能
  • その他

Levels of review

  • Level1: 即ApproveしてOK
  • Level2: 軽く目を通して問題がなければApprove
  • Level3: 実際にビルドして動作確認をしてほしい

@tinaxd
Copy link
Member

tinaxd commented Apr 11, 2023

いいですね!
Firefox でも複数デバイス同期が動作することを確認しました

@das08
Copy link
Member

das08 commented Apr 14, 2023

めちゃくちゃいいですね!
確認して大きな問題がなければアップデートします〜

src/features/storage/index.ts Outdated Show resolved Hide resolved
src/features/storage/index.ts Outdated Show resolved Hide resolved
src/features/storage/index.ts Outdated Show resolved Hide resolved
@das08
Copy link
Member

das08 commented Apr 14, 2023

storage.localstorage.syncを共存させる案に私も賛成ですが,初めてsyncをONにした場合,storage.syncが空っぽなのでもともとsync OFF時にあったチェックボックスやメモ,設定が引き継がれないという問題が発生してしまいますね.
storage.localstorage.syncを"同期"するのがベストですがめんどくさそうな気もします

@das08 das08 self-assigned this Apr 14, 2023
@das08 das08 added the enhancement New feature or request label Apr 14, 2023
@tinaxd
Copy link
Member

tinaxd commented Apr 14, 2023

設定をONに変更したとき:syncの中身が空の場合(≒同期機能初使用時)に限りlocalからsyncにデータをコピー
設定をOFFに変更したとき:syncからlocalにデータをコピー
常に同期しなくても、これでいい感じにデータを引き継げないでしょうか

@walnuts1018
Copy link
Contributor Author

walnuts1018 commented Apr 14, 2023

おっしゃる通りです。localとsyncを切り替える時に初期化されてしまいます。

SettingsのOnChange(?)にtinaxdさんのおっしゃた通りの処理を書こうと思ったのですが、動作がよく分かっていなかった(&じっくり読むのをサボってしまった)ので、syncのオンオフは頻繁に切り替えないだろうという想定の下、リセットを許容することにしてしまいました。

消えるのは課題のチェックぐらいでそんなに困らないだろうだと思っていましたが、そういえばmemoも消えてしまうんでしたね。それは消えると困りそうです。修正してみます。

@walnuts1018 walnuts1018 marked this pull request as draft April 26, 2023 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants