Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions modules/study/src/main/StudyApi.scala
Original file line number Diff line number Diff line change
Expand Up @@ -468,15 +468,17 @@ final class StudyApi(
doSetClock(Study.WithChapter(study, c), Position(c, UciPath.root).ref, clock)(who)
yield sendTo(study.id)(_.setTags(chapter.id, chapter.tags, who))

def setComment(studyId: StudyId, position: Position.Ref, text: CommentStr)(who: Who) =
def setComment(studyId: StudyId, position: Position.Ref, commentId: Option[Comment.Id], text: CommentStr)(
who: Who
) =
sequenceStudyWithChapter(studyId, position.chapterId):
case Study.WithChapter(study, chapter) =>
Contribute(who.u, study):
lightUserApi
.async(who.u)
.flatMapz: author =>
val comment = Comment(
id = Comment.Id.make,
id = commentId.getOrElse(Comment.Id.make),
text = text,
by = Comment.Author.User(author.id, author.titleName)
)
Expand All @@ -486,8 +488,8 @@ final class StudyApi(
position.chapter.setComment(comment, position.path) match
case Some(newChapter) =>
newChapter.root.nodeAt(position.path).so { node =>
node.comments.findBy(comment.by).so { c =>
for _ <- chapterRepo.setComments(node.comments.filterEmpty)(newChapter, position.path)
node.comments.findByIdAndAuthor(comment.id, comment.by).so { c =>
for _ <- chapterRepo.setComments(node.comments.set(c))(newChapter, position.path)
yield
sendTo(study.id)(_.setComment(position.ref, c, who))
studyRepo.updateNow(study)
Expand Down
3 changes: 2 additions & 1 deletion modules/study/src/main/StudySocket.scala
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ final private class StudySocket(
(o \ "d" \ "text")
.asOpt[String]
.foreach: text =>
applyWho(api.setComment(studyId, position.ref, Comment.sanitize(text)))
val commentId = (o \ "d" \ "id").asOpt[String].map(Comment.Id.apply)
applyWho(api.setComment(studyId, position.ref, commentId, Comment.sanitize(text)))

case "deleteComment" =>
reading[AtPosition](o): position =>
Expand Down
4 changes: 2 additions & 2 deletions modules/study/src/test/StudyIntegrationTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -267,14 +267,14 @@ object Fixtures:
""".trim

val pgn4 =
"1. e4 e6 2. d4 d5 3. Nc3 (3. exd5 exd5!! $15 { We have French exchange, the most exciting opening ever } 4. Bd3) (3. e5 c5 { French Defence: Advance Variation, another better position for Black } { [%cal Gc5d4,Gh2h4] }) 3... dxe4 { 3. Nc3 is the main weapon of White, but it doesn't match for the powerful Rubinstein. White is screwed here } (3... Bb4) 4. Nxe4 Nd7"
"1. e4 e6 2. d4 d5 3. Nc3 (3. exd5 exd5!! $15 { We have French exchange, the most exciting opening ever } 4. Bd3) (3. e5 c5 { French Defence: Advance Variation, another better position for Black } { [%cal Gc5d4,Gh2h4] }) 3... 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 } (3... Bb4) 4. Nxe4 Nd7"

val m5 = s"""$m4\n{"t":"clearAnnotations","d":"kMOZO15F"}"""
val pgn5 = "1. e4 e6 2. d4 d5 3. Nc3 (3. exd5 exd5 4. Bd3) (3. e5 c5) 3... dxe4 (3... Bb4) 4. Nxe4 Nd7"

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


val ms = List(m0, m1, m2, m3, m4, m5, m6)
val ps = List(pgn0, pgn1, pgn2, pgn3, pgn4, pgn5, pgn6)
Expand Down
8 changes: 6 additions & 2 deletions modules/tree/src/main/tree.scala
Original file line number Diff line number Diff line change
Expand Up @@ -479,11 +479,15 @@ object Node:
opaque type Comments = List[Comment]
object Comments extends TotalWrapper[Comments, List[Comment]]:
extension (a: Comments)
def findById(id: Comment.Id) = a.value.find(_.id == id)
def findBy(author: Comment.Author) = a.value.find(_.by.is(author))
def findByIdAndAuthor(id: Comment.Id, author: Comment.Author) =
a.value.find(c => c.id == id && c.by.is(author))
def set(comment: Comment): Comments =
if a.value.exists(_.by.is(comment.by)) then
if a.value.exists(c => c.by.is(comment.by) && c.id == comment.id) then
a.value.map:
case c if c.by.is(comment.by) => c.copy(text = comment.text, by = comment.by)
case c if c.by.is(comment.by) && c.id == comment.id =>
c.copy(text = comment.text, by = comment.by)
case c => c
else a.value :+ comment
def delete(commentId: Comment.Id): Comments = a.value.filterNot(_.id == commentId)
Expand Down
2 changes: 1 addition & 1 deletion ui/analyse/src/socket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export interface StudySocketSendParams {
promote: (d: ReqPosition & { toMainline: boolean }) => void;
forceVariation: (d: ReqPosition & { force: boolean }) => void;
shapes: (d: ReqPosition & { shapes: Tree.Shape[] }) => void;
setComment: (d: ReqPosition & { text: string }) => void;
setComment: (d: ReqPosition & { id?: string; text: string }) => void;
deleteComment: (d: ReqPosition & { id: string }) => void;
setGamebook: (d: ReqPosition & { gamebook: { deviation?: string; hint?: string } }) => void;
toggleGlyph: (d: ReqPosition & { id: number }) => void;
Expand Down
129 changes: 98 additions & 31 deletions ui/analyse/src/study/commentForm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,62 +10,86 @@ interface Current {
chapterId: string;
path: Tree.Path;
node: Tree.Node;
commentId: string;
focusId?: string;
}

export class CommentForm {
current = prop<Current | null>(null);
opening = prop(false);
currents = new Map<string, Current>();
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

this.newComment = newComment;
};

constructor(readonly root: AnalyseCtrl) {}

submit = (text: string) => this.current() && this.doSubmit(text);
makeKey(chapterId: string, path: Tree.Path, commentId: string): string {
return `${chapterId}:${path}:${commentId}`;
}

submit = (key: string, el: HTMLInputElement) => {
const current = this.currents.get(key);
if (current) {
this.currents.forEach((cur, _) => {
cur.focusId = undefined;
});
this.currents.get(key)!.focusId = el.id;
this.doSubmit(current.chapterId, current.path, current.commentId, el.value);
}
};

doSubmit = throttle(500, (text: string) => {
const cur = this.current();
if (cur) this.root.study!.makeChange('setComment', { ch: cur.chapterId, path: cur.path, text });
doSubmit = throttle(500, (chapterId: string, path: Tree.Path, commentId: string, text: string) => {
this.root.study!.makeChange('setComment', { ch: chapterId, path, id: commentId, text });
});

start = (chapterId: string, path: Tree.Path, node: Tree.Node): void => {
this.opening(true);
this.current({ chapterId, path, node });
start = (chapterId: string, path: Tree.Path, node: Tree.Node, commentId: string): void => {
const key = this.makeKey(chapterId, path, commentId);
this.opening(key);
this.currents.set(key, { chapterId, path, node, commentId });
this.root.userJump(path);
};

onSetPath = (chapterId: string, path: Tree.Path, node: Tree.Node): void => {
const cur = this.current();
if (cur && (path !== cur.path || chapterId !== cur.chapterId || cur.node !== node)) {
cur.chapterId = chapterId;
cur.path = path;
cur.node = node;
}
this.currents.forEach(cur => {
if (path !== cur.path || chapterId !== cur.chapterId || cur.node !== node) {
cur.chapterId = chapterId;
cur.path = path;
cur.node = node;
}
});
};

delete = (chapterId: string, path: Tree.Path, id: string) => {
this.root.study!.makeChange('deleteComment', { ch: chapterId, path, id });
};

clear = () => {
this.currents.clear();
};
}

export const viewDisabled = (root: AnalyseCtrl, why: string): VNode =>
h('div.study__comments', [currentComments(root, true), h('div.study__message', why)]);

export function view(root: AnalyseCtrl): VNode {
const study = root.study!,
ctrl = study.commentForm,
current = ctrl.current();
if (!current) return viewDisabled(root, 'Select a move to comment');
function renderTextarea(root: AnalyseCtrl, ctrl: CommentForm, current: Current, key: string): VNode {
const study = root.study!;
const setupTextarea = (vnode: VNode, old?: VNode) => {
const el = vnode.elm as HTMLInputElement;
const newKey = current.chapterId + current.path;

if (old?.data!.path !== newKey) {
const el = vnode.elm as HTMLTextAreaElement;
if (old?.data!.key !== key) {
const mine = (current.node.comments || []).find(function (c) {
return isAuthorObj(c.by) && c.by.id && c.by.id === ctrl.root.opts.userId;
return (
isAuthorObj(c.by) && c.by.id && c.by.id === ctrl.root.opts.userId && c.id === current.commentId
);
});
el.value = mine ? mine.text : '';
}
vnode.data!.path = newKey;
vnode.data!.key = key;

if (ctrl.opening()) {
if (ctrl.opening() === key) {
requestAnimationFrame(() => el.focus());
ctrl.opening(false);
ctrl.opening(null);
}
};

Expand All @@ -75,12 +99,12 @@ export function view(root: AnalyseCtrl): VNode {
[
currentComments(root, !study.members.canContribute()),
h('form.form3', [
h('textarea#comment-text.form-control', {
h(`textarea#${key}.form-control`, {
hook: {
insert(vnode) {
setupTextarea(vnode);
const el = vnode.elm as HTMLInputElement;
el.oninput = () => setTimeout(() => ctrl.submit(el.value), 50);
el.oninput = () => setTimeout(() => ctrl.submit(key, el), 50);
const heightStore = storage.make('study.comment.height');
el.onmouseup = () => heightStore.set('' + el.offsetHeight);
el.style.height = parseInt(heightStore.get() || '80') + 'px';
Expand All @@ -89,10 +113,53 @@ export function view(root: AnalyseCtrl): VNode {
if (e.code === 'Escape') el.blur();
});
},
postpatch: (old, vnode) => setupTextarea(vnode, old),
postpatch: (old, vnode) => {
setupTextarea(vnode, old);
const el = vnode.elm as HTMLInputElement;
if (ctrl.currents.get(key)?.focusId === el.id) el.focus();
el.oninput = () => setTimeout(() => ctrl.submit(key, el), 50);
},
},
}),
]),
],
);
}

export function view(root: AnalyseCtrl): VNode {
const study = root.study!;
const commForm = study.commentForm;

const commentKeys = new Set(
(root.node.comments || []).map(c => commForm.makeKey(study.vm.chapterId, root.path, c.id)),
);

const rerender = Array.from(commForm.currents.keys()).some(key => !commentKeys.has(key));
const comments = root.node.comments || [];
if (rerender || commForm.currents.size === 0) {
if (!commForm.newComment) commForm.clear();
commForm.setnewComment(false);
comments.forEach(comment => {
commForm.start(study.vm.chapterId, root.path, root.node, comment.id);
});
}

if (commForm.currents.size === 0) {
return viewDisabled(root, 'Select a move to comment');
}

return h(
'div.study__comments',
{
hook: onInsert(() => root.enableWiki(root.data.game.variant.key === 'standard')),
},
[
h(
'div.study__comment-forms',
Array.from(commForm.currents.entries()).map(([key, current]) => {
return renderTextarea(root, commForm, current, key);
}),
),
h('div.analyse__wiki.study__wiki.force-ltr'),
],
);
Expand Down
2 changes: 1 addition & 1 deletion ui/analyse/src/study/gamebook/gamebookEdit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export function render(ctrl: AnalyseCtrl): VNode {
const commentHook: Hooks = bind(
'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.

study.vm.toolTab('comments');
requestIdleCallback(
() =>
Expand Down
9 changes: 7 additions & 2 deletions ui/analyse/src/study/studyCtrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -592,13 +592,18 @@ export default class StudyCtrl {
);
toggleSticky = () => {
this.vm.mode.sticky = !this.vm.mode.sticky && this.data.features.sticky;
this.xhrReload();
// Only reload if actually changing sticky mode
if (this.vm.mode.sticky) {
this.xhrReload();
} else {
this.redraw();
}
};
toggleWrite = () => {
this.vm.mode.write = !this.vm.mode.write && this.members.canContribute();
if (this.relay) this.relayRecProp(this.vm.mode.write);
else this.nonRelayRecMapProp(this.data.id, this.vm.mode.write);
this.xhrReload();
this.redraw();
};
goToPrevChapter = () => {
const chapter = this.prevChapter();
Expand Down
7 changes: 5 additions & 2 deletions ui/analyse/src/study/studyView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ export function contextMenu(ctrl: StudyCtrl, path: Tree.Path, node: Tree.Node):
attrs: dataIcon(licon.BubbleSpeech),
hook: bind('click', () => {
ctrl.vm.toolTab('comments');
ctrl.commentForm.start(ctrl.currentChapter().id, path, node);
ctrl.commentForm.setnewComment(true);
ctrl.commentForm.start(ctrl.currentChapter().id, path, node, '');
}),
},
i18n.study.commentThisMove,
Expand Down Expand Up @@ -260,7 +261,9 @@ function buttons(root: AnalyseCtrl): VNode {
hint: i18n.study.commentThisPosition,
icon: iconTag(licon.BubbleSpeech),
onClick() {
ctrl.commentForm.start(ctrl.vm.chapterId, root.path, root.node);
root.node.comments?.forEach(comment =>
ctrl.commentForm.start(ctrl.vm.chapterId, root.path, root.node, comment.id),
);
},
count: (root.node.comments || []).length,
}),
Expand Down