Skip to content

Conversation

@dhalleela
Copy link
Contributor

Changes to fix #18467.

Implemented necessary, albeit slightly intricate, focus management logic to get the focus to land on the correct text area within commentForm.ts. This correctly directs the focus to the recently edited text area, preventing it from jumping to the last text area.

_.withChildren: children =>
if toMainline then children.promoteToMainlineAt(position.path)
else children.promoteUpAt(position.path)._1F
else children.promoteUpAt(position.path).map(_._1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

undoing unrelated previous commits?

Copy link
Contributor Author

@dhalleela dhalleela Dec 16, 2025

Choose a reason for hiding this comment

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

Thanks @ornicar.unrelated previous commits were restored.

(newChapter.description != chapter.description)
if shouldReload then sendTo(study.id)(_.updateChapter(chapter.id, who))
else reloadChapters(study)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

undoing unrelated previous commits?

Copy link
Contributor Author

@dhalleela dhalleela Dec 16, 2025

Choose a reason for hiding this comment

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

Thanks @ornicar.unrelated previous commits were restored.

@ornicar
Copy link
Collaborator

ornicar commented Dec 14, 2025

CI shows that study tests are broken

@dhalleela
Copy link
Contributor Author

task is complete

opening = prop<string | null>(null);
newComment = false;

setnewComment = (newComment: boolean) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

namingConventions

@ornicar
Copy link
Collaborator

ornicar commented Dec 16, 2025

This looks rather broken.

Import {test 1 } {test 2} 1.d4 {test 3} { test 4}.

The comment section is very confused:

image

And typing in the textarea makes things worse.

I think this approach is flawed by complexity. It will take too much time and code to make it work properly, and much more to maintain over the years.

We should be looking for a much simpler solution, like maybe concatenating multiple comments into one when importing PGN.

'click',
() => {
study.commentForm.start(study.vm.chapterId, ctrl.path, ctrl.node);
// ctrl.node.comments?.forEach((comment) => ctrl.study!.commentForm.start(ctrl.study!.vm.chapterId, ctrl.path, ctrl.node, comment.id));
Copy link
Collaborator

Choose a reason for hiding this comment

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

dead code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a valid question.
going to experiment with concatenation approach to see if it can make code more stable for long term viability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#18467
I used steps to reproduce and created a chapter. PGN & comments testing passed.

But with -
import {test 1 } {test 2} 1.d4 {test 3} { test 4} the comment section rendered confusing. I have to fix this rendering.

I think I will be able refine code to be less complex.

val m6 = s"""$m4\n{"t":"clearVariations","d":"kMOZO15F"}"""
val pgn6 =
"1. e4 e6 2. d4 d5 3. Nc3 dxe4 { 3. Nc3 is the main weapon of White, but it doesn't match for the powerful Rubinstein. White is screwed here } 4. Nxe4 Nd7"
"1. e4 e6 2. d4 d5 3. Nc3 dxe4 { 3. Nc3 is the main weapon of White, but it doesn't match for the powerful Rubinstein. White is scrwed } { 3. Nc3 is the main weapon of White, but it doesn't match for the powerful Rubinstein. White is screwed here } 4. Nxe4 Nd7"
Copy link
Collaborator

Choose a reason for hiding this comment

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

so these tests show that without the comment ID, each modification to a comment adds a new comment.

it looks like a good place for tests that show how comment IDs work

@dhalleela dhalleela requested a review from ornicar December 16, 2025 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Study import: multiple comments cannot be edited

2 participants