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

fix: Do not insert initial value when input is empty in editor #8773

Conversation

miya
Copy link
Member

@miya miya commented Apr 30, 2024

Task

#126535 [v7][New Editor][autosave] 明示的なページの保存でオートセーブリビジョンが自動で削除される
#130798 空文字にしたときに initialValue が入らないようにする

@miya miya requested a review from yuki-takei April 30, 2024 07:30
@miya miya self-assigned this Apr 30, 2024
Base automatically changed from feat/confirm-editor-is-active to master May 8, 2024 21:04
@@ -92,7 +93,9 @@ export const useCollaborativeEditorMode = (

socketIOProvider.on('sync', (isSync: boolean) => {
if (isSync) {
socket.emit(GlobalSocketEventName.YDocSync, { pageId, initialValue });
// If no draft exists, insert initial value
socket.emit(GlobalSocketEventName.YDocSync, { pageId, initialValue: hasDraft ? undefined : initialValue });
Copy link
Member Author

Choose a reason for hiding this comment

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

ドラフトが存在しない時に初期値を入れるようにした (ドラフトを空文字にしても初期値が入らない)

@@ -192,6 +192,10 @@ class SocketIoService {
});

socket.on(GlobalSocketEventName.YDocSync, async({ pageId, initialValue }) => {
this.io
.in(getRoomNameWithId(RoomPrefix.PAGE, pageId))
.emit('hasYjsDraft', true);
Copy link
Member Author

Choose a reason for hiding this comment

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

Editor を開いたら、ドラフトが生成されたことをクライアントに通知する


const { data: codeMirrorEditor } = useCodeMirrorEditorIsolated(GlobalCodeMirrorEditorKey.MAIN);

const [markdownToPreview, setMarkdownToPreview] = useState<string>(codeMirrorEditor?.getDoc() ?? '');
Copy link
Member Author

Choose a reason for hiding this comment

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

  • markdownToPreview の初期値にエディターで入力中の文字列を入れるようにした
  • ドラフトが空文字列の時に CodeMirrorEditorMain に渡す onChange がトリガーされたかったためこのような改修を行なった

@yuki-takei
Copy link
Member

https://redmine.weseek.co.jp/issues/130798 に書いてある前提・背景のためだけにこれだけのコードや追加の store やイベントを追加するのはあまりやりたくないな

@@ -62,7 +62,7 @@ class YjsConnectionManager {
const persistedCodeMirrorText = persistedYdoc.getText('codemirror').toString();
const currentCodeMirrorText = currentYdoc.getText('codemirror').toString();

if (persistedCodeMirrorText === '' && currentCodeMirrorText === '') {
if (persistedCodeMirrorText === '' && currentCodeMirrorText === '' && initialValue != null) {
Copy link
Member

Choose a reason for hiding this comment

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

getYdoc() や getText で得られるインスタンスにはそれなりに多くの情報が入っているように見える。
toString() した結果だけをもって判定しようとするから融通が利かないのでは?

Copy link
Member Author

Choose a reason for hiding this comment

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

こちらで解決しました
#8773 (comment)

@miya miya removed the type/feature label May 23, 2024
if (persistedCodeMirrorText === '' && currentCodeMirrorText === '') {
// If no write operation has been performed, insert initial value
const clientsSize = currentYdoc.store.clients.size;
if (clientsSize === 0) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Ydoc の初期化後に何らかの操作を行うことにより store.clients.size が増えていく

参考

https://speakerdeck.com/kentomoriwaki/wakatutaqi-ninareru-crdt-woshi-tutagong-tong-bian-ji?slide=22

Copy link

reg-suit bot commented May 23, 2024

reg-suit detected visual differences.

Check this report, and review them.

🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴

🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵

What do the circles mean? The number of circles represent the number of changed images.
🔴 : Changed items, ⚪ : New items, ⚫ : Deleted items, and 🔵 Passed items

How can I change the check status? If reviewers approve this PR, the reg context status will be green automatically.

@yuki-takei yuki-takei merged commit 1620dcd into master May 24, 2024
21 of 24 checks passed
@yuki-takei yuki-takei deleted the feat/130798-do-not-insert-initial-value-when-input-is-empty-in-editor branch May 24, 2024 02:13
@github-actions github-actions bot mentioned this pull request May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants