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

Go to move which sender was looking at when message was sent in broadcasts #15570

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
221cda1
Update chatCtrl to include round and game information in chat messages
Carbrex Jun 17, 2024
3547b56
Merge branch 'lichess-org:master' into broadcast-chat-game-navigate-1…
Carbrex Jun 20, 2024
70c82d3
remove site.analysis and move code to analyse module (#2)
Carbrex Jul 6, 2024
2caefd0
remove comments
Carbrex Jul 6, 2024
f7155e1
Fix lint errors
Carbrex Jul 6, 2024
233373e
Change function names and fix ui when report button is also visible
Carbrex Jul 6, 2024
3d356b6
Merge branch 'master' into broadcast-chat-game-navigate-15029
Carbrex Jul 6, 2024
4bf6e75
Fix jump not taking to correct move if move is of different chapter
Carbrex Jul 6, 2024
13636b7
Merge branch 'master' into broadcast-chat-game-navigate-15029
ornicar Jul 7, 2024
3e43346
move relay chat code to the relay dir
ornicar Jul 7, 2024
0031e1d
fix repeatedly hardcoded relay chat separator
ornicar Jul 7, 2024
4c8aced
ui/chat cannot know about relay and hardcode its separator
ornicar Jul 7, 2024
02ed544
move more relay code to the relay dir
ornicar Jul 7, 2024
accf84c
Jump to correct move only if chapter has loaded
Carbrex Jul 7, 2024
9ff3d6e
Don't add metadata if text length exceeds 140 after encoding and don'…
Carbrex Jul 7, 2024
d124f2b
Use studyvm to go to a given move after chapter is fetched
Carbrex Jul 9, 2024
cdadf9a
Merge branch 'master' into broadcast-chat-game-navigate-15029
ornicar Jul 10, 2024
eda345a
ui/common throttlePromise now returns a Promise for chaining
ornicar Jul 10, 2024
1a77d80
pnpm lint fixes
ornicar Jul 10, 2024
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
26 changes: 18 additions & 8 deletions modules/common/src/main/String.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,28 @@ object String:
try play.utils.UriEncoding.decodePath(input, "UTF-8").some
catch case _: play.utils.InvalidUriEncodingException => None

def isShouting(text: String) =
def isShouting(text: String): Boolean =
text.lengthIs >= 5 && {
import java.lang.Character.*
// true if >1/2 of the latin letters are uppercase
text.take(80).replace("O-O", "o-o").foldLeft(0) { (i, c) =>
getType(c) match
case UPPERCASE_LETTER => i + 1
case LOWERCASE_LETTER => i - 1
case _ => i
} > 0
if text.contains('\ue666') then
Copy link
Collaborator

@ornicar ornicar Jul 10, 2024

Choose a reason for hiding this comment

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

No. The broadcast encoding logic must not spill over unrelated section of the code, such as this one.

isShouting must not even assume it's working on a chat message.

Copy link
Member Author

Choose a reason for hiding this comment

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

What is your suggested way? Do I add 128 to ascii value of each character of chapterId in encodeMsg and then get back the chapterId by subtracting 128? So the chapterId doesn't affect the shouting mechanism.

Copy link
Member Author

Choose a reason for hiding this comment

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

let encodedChapterId = '';
for (let i = 0; i < chapterId.length; i++) {
  encodedChapterId += String.fromCharCode(chapterId.charCodeAt(i) + 128);
}
chapterId = encodedChapterId;

Something like this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

isShouting should only be applied to the text that was actually typed by the user.

val (before, after) = text.span(_ != '\ue666')
isShouting(before)
else
text.take(80).replace("O-O", "o-o").foldLeft(0) { (i, c) =>
getType(c) match
case UPPERCASE_LETTER => i + 1
case LOWERCASE_LETTER => i - 1
case _ => i
} > 0
}
def noShouting(str: String): String = if isShouting(str) then str.toLowerCase else str
def noShouting(str: String): String = if isShouting(str) then
// '\ue666' is a special character used to encode broadcast chat messages. See file://./../../../../ui/analyse/src/study/relay/chatHandler.ts
if str.contains('\ue666') then
val (before, after) = str.span(_ != '\ue666')
before.toLowerCase + after
else str.toLowerCase
else str

val atUsernameRegex = RawHtml.atUsernameRegex
val forumPostPathRegex = """(?:(?<= )|^)\b([\w-]+/[\w-]+)\b(?:(?= )|$)""".r
Expand Down
2 changes: 2 additions & 0 deletions ui/analyse/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { AnalyseSocketSend } from './socket';
import { ExternalEngineInfo } from 'ceval';
import * as Prefs from 'common/prefs';
import { EnhanceOpts } from 'common/richText';
import { BroadcastChatHandler } from 'chat/src/interfaces';

export type Seconds = number;

Expand Down Expand Up @@ -158,6 +159,7 @@ export interface AnalyseOpts {
chat: {
enhance: EnhanceOpts;
instance?: Promise<ChatCtrl>;
broadcastChatHandler?: BroadcastChatHandler;
};
wiki?: boolean;
inlinePgn?: string;
Expand Down
74 changes: 74 additions & 0 deletions ui/analyse/src/study/relay/chatHandler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { BroadcastChatHandler, Line } from 'chat/src/interfaces';
import AnalyseCtrl from '../../ctrl';
import { VNode, h } from 'snabbdom';
import { bind } from 'common/snabbdom';

export function broadcastChatHandler(ctrl: AnalyseCtrl): BroadcastChatHandler {
// '\ue666' was arbitrarily chosen from the unicode private use area to separate the text from the chapterId and ply
const separator = '\ue666';

const encodeMsg = (text: string): string => {
text = cleanMsg(text);
if (ctrl.study?.relay && !ctrl.study.relay.tourShow()) {
const chapterId = ctrl.study.currentChapter().id;
const ply = ctrl.study.currentNode().ply;
const newText = text + separator + chapterId + separator + ply;
if (newText.length <= 140) {
text = newText;
}
}
return text;
};

const cleanMsg = (msg: string): string => {
if (msg.includes(separator) && ctrl.study?.relay) {
return msg.split(separator)[0];
}
return msg;
};

const jumpToMove = async (msg: string) => {
if (msg.includes(separator) && ctrl.study?.relay) {
const segs = msg.split(separator);
if (segs.length == 3) {
const [, chapterId, ply] = segs;
await ctrl.study.setChapter(chapterId);
ctrl.jumpToMain(parseInt(ply));
}
}
};

const canJumpToMove = (msg: string): string | null => {
if (msg.includes(separator) && ctrl.study?.relay) {
const segs = msg.split(separator);
if (segs.length == 3) {
const [, chapterId, ply] = segs;
return `${chapterId}#${ply}`;
}
}
return null;
};

const jumpButton = (line: Line): VNode | null => {
const msgPly = canJumpToMove(line.t);
return msgPly
? h(
'button.jump',
{
hook: bind('click', () => jumpToMove(line.t)),
attrs: {
title: `Jump to move ${msgPly}`,
},
},
'#',
)
: null;
};

return {
encodeMsg,
cleanMsg,
jumpToMove,
jumpButton,
};
}
2 changes: 1 addition & 1 deletion ui/analyse/src/study/relay/relayCtrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default class RelayCtrl {
private readonly chapters: StudyChapters,
private readonly multiCloudEval: MultiCloudEval,
private readonly federations: () => Federations | undefined,
setChapter: (id: ChapterId | number) => boolean,
setChapter: (id: ChapterId | number) => Promise<void>,
) {
this.tourShow = toggle((location.pathname.match(/\//g) || []).length < 5);
const locationTab = location.hash.replace(/^#/, '') as RelayTab;
Expand Down
2 changes: 1 addition & 1 deletion ui/analyse/src/study/relay/relayTeams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default class RelayTeams {
constructor(
private readonly roundId: RoundId,
readonly multiCloudEval: MultiCloudEval,
readonly setChapter: (id: ChapterId | number) => boolean,
readonly setChapter: (id: ChapterId | number) => Promise<void>,
readonly roundPath: () => string,
private readonly redraw: Redraw,
) {}
Expand Down
7 changes: 5 additions & 2 deletions ui/analyse/src/study/studyChapters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,18 @@ export function resultOf(tags: TagArray[], isWhite: boolean): string | undefined
export const gameLinkAttrs = (basePath: string, game: { id: ChapterId }) => ({
href: `${basePath}/${game.id}`,
});
export const gameLinksListener = (setChapter: (id: ChapterId | number) => boolean) => (vnode: VNode) =>
export const gameLinksListener = (setChapter: (id: ChapterId | number) => Promise<void>) => (vnode: VNode) =>
(vnode.elm as HTMLElement).addEventListener(
'click',
e => {
let target = e.target as HTMLLinkElement;
while (target && target.tagName !== 'A') target = target.parentNode as HTMLLinkElement;
const href = target?.href;
const id = target?.dataset['board'] || href?.match(/^[^?#]*/)?.[0].slice(-8);
if (id && setChapter(id) && !href?.match(/[?&]embed=/)) e.preventDefault();
if (id && !href?.match(/[?&]embed=/)) {
setChapter(id);
e.preventDefault();
}
},
{ passive: false },
);
Expand Down
19 changes: 10 additions & 9 deletions ui/analyse/src/study/studyCtrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ export default class StudyCtrl {
this.updateAddressBar();
};

xhrReload = throttlePromiseDelay(
xhrReload: () => Promise<void> = throttlePromiseDelay(
() => 500,
() => {
this.vm.loading = true;
Expand Down Expand Up @@ -448,31 +448,32 @@ export default class StudyCtrl {

likeToggler = debounce(() => this.send('like', { liked: this.data.liked }), 1000);

setChapter = (idOrNumber: ChapterId | number, force?: boolean): boolean => {
setChapter = (idOrNumber: ChapterId | number, force?: boolean): Promise<void> => {
const prev = this.chapters.list.get(idOrNumber);
const id = prev?.id;
if (!id) {
console.warn(`Chapter ${idOrNumber} not found`);
return false;
return Promise.reject();
}
const alreadySet = id === this.vm.chapterId && !force;
if (this.relay?.tourShow()) {
this.relay.tourShow(false);
if (alreadySet) this.redraw();
}
if (alreadySet) return true;
let result = Promise.resolve();
if (alreadySet) return result;
if (!this.vm.mode.sticky || !this.makeChange('setChapter', id)) {
this.vm.mode.sticky = false;
if (!this.vm.behind) this.vm.behind = 1;
this.vm.chapterId = id;
this.xhrReload();
result = this.xhrReload();
}
this.vm.loading = true;
this.vm.nextChapterId = id;
this.vm.justSetChapterId = id;
this.redraw();
window.scrollTo(0, 0);
return true;
return result;
};

private deltaChapter = (delta: number): ChapterPreview | undefined => {
Expand Down Expand Up @@ -638,7 +639,7 @@ export default class StudyCtrl {
(position.path === this.ctrl.path && position.path === treePath.fromNodeList(this.ctrl.mainline))
)
this.ctrl.jump(newPath);
this.redraw();
return this.redraw();
},
deleteNode: d => {
const position = d.p,
Expand All @@ -650,7 +651,7 @@ export default class StudyCtrl {
if (!this.ctrl.tree.pathExists(d.p.path)) return this.xhrReload();
this.ctrl.tree.deleteNodeAt(position.path);
if (this.vm.mode.sticky) this.ctrl.jump(this.ctrl.path);
this.redraw();
return this.redraw();
},
promote: d => {
const position = d.p,
Expand All @@ -663,7 +664,7 @@ export default class StudyCtrl {
if (this.vm.mode.sticky) this.ctrl.jump(this.ctrl.path);
else if (this.relay) this.ctrl.jump(d.p.path);
this.ctrl.treeVersion++;
this.redraw();
return this.redraw();
},
reload: this.xhrReload,
changeChapter: d => {
Expand Down
2 changes: 2 additions & 0 deletions ui/analyse/src/view/components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import StudyCtrl from '../study/studyCtrl';
import RelayCtrl from '../study/relay/relayCtrl';
import type * as studyDeps from '../study/studyDeps';
import { renderPgnError } from '../pgnImport';
import { broadcastChatHandler } from '../study/relay/chatHandler';

export interface ViewContext {
ctrl: AnalyseCtrl;
Expand Down Expand Up @@ -389,6 +390,7 @@ export function makeChat(ctrl: AnalyseCtrl, insert: (chat: HTMLElement) => void)
chatEl.classList.add('mchat');
insert(chatEl);
const chatOpts = ctrl.opts.chat;
chatOpts.broadcastChatHandler = broadcastChatHandler(ctrl);
chatOpts.instance?.then(c => c.destroy());
chatOpts.enhance = { plies: true, boards: !!ctrl.study?.relay };
chatOpts.instance = site.makeChat(chatOpts);
Expand Down
28 changes: 23 additions & 5 deletions ui/chat/css/_discussion.scss
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,7 @@

action {
display: none;
position: absolute;
top: 5px;
@include inline-end(0);
cursor: pointer;
margin-inline-end: 3px;
padding: 1px 5px;
opacity: 0.7;
color: $c-accent;

Expand Down Expand Up @@ -111,6 +106,29 @@
}
}

button.jump {
font-size: medium;
cursor: pointer;
background-color: transparent;
color: $c-link;
border: none;
}

button.jump:hover {
color: $c-link-hover;
}

div.actions {
position: absolute;
@include inline-end(0);
display: flex;
flex-direction: row;
top: 5px;
gap: 6px;
margin-inline-end: 3px;
padding: 1px 5px;
}

&__say {
flex: 0 0 auto;
border: 0;
Expand Down
5 changes: 5 additions & 0 deletions ui/chat/src/ctrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
ChatData,
NoteCtrl,
ChatPalantir,
BroadcastChatHandler,
} from './interfaces';
import { PresetCtrl, presetCtrl } from './preset';
import { noteCtrl } from './note';
Expand All @@ -30,12 +31,14 @@ export default class ChatCtrl {
preset: PresetCtrl;
trans: Trans;
vm: ViewModel;
broadcastChatHandler: BroadcastChatHandler;

constructor(
readonly opts: ChatOpts,
readonly redraw: Redraw,
) {
this.data = opts.data;
this.broadcastChatHandler = opts.broadcastChatHandler;
if (opts.noteId) this.allTabs.push('note');
if (opts.plugin) this.allTabs.push(opts.plugin.tab.key);
this.palantir = {
Expand Down Expand Up @@ -102,6 +105,8 @@ export default class ChatCtrl {
alert('Max length: 140 chars. ' + text.length + ' chars used.');
return false;
}
if (this.broadcastChatHandler) text = this.broadcastChatHandler.encodeMsg(text);

site.pubsub.emit('socket.send', 'talk', text);
return true;
};
Expand Down
19 changes: 12 additions & 7 deletions ui/chat/src/discussion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ const userThunk = (name: string, title?: string, patron?: boolean, flair?: Flair
userLink({ name, title, patron, line: !!patron, flair });

function renderLine(ctrl: ChatCtrl, line: Line): VNode {
const textNode = renderText(line.t, ctrl.opts.enhance);
const textNode = renderText(ctrl.broadcastChatHandler?.cleanMsg(line.t) || line.t, ctrl.opts.enhance);

if (line.u === 'lichess') return h('li.system', textNode);

Expand All @@ -204,6 +204,8 @@ function renderLine(ctrl: ChatCtrl, line: Line): VNode {
.match(enhance.userPattern)
?.find(mention => mention.trim().toLowerCase() == `@${ctrl.data.userId}`);

const jumpButton = ctrl.broadcastChatHandler?.jumpButton(line);

return h(
'li',
{
Expand All @@ -214,13 +216,16 @@ function renderLine(ctrl: ChatCtrl, line: Line): VNode {
},
},
ctrl.moderation
? [line.u ? modLineAction() : null, userNode, ' ', textNode]
? [h('div.actions', [line.u ? modLineAction() : null, jumpButton]), userNode, ' ', textNode]
: [
myUserId && line.u && myUserId != line.u
? h('action.flag', {
attrs: { 'data-icon': licon.CautionTriangle, title: 'Report' },
})
: null,
h('div.actions', [
myUserId && line.u && myUserId != line.u
? h('action.flag', {
attrs: { 'data-icon': licon.CautionTriangle, title: 'Report' },
})
: null,
jumpButton,
]),
userNode,
' ',
textNode,
Expand Down
8 changes: 8 additions & 0 deletions ui/chat/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export interface ChatOpts {
blind: boolean;
timeout: boolean;
enhance?: EnhanceOpts;
broadcastChatHandler: BroadcastChatHandler;
public: boolean;
permissions: Permissions;
timeoutReasons?: ModerationReason[];
Expand Down Expand Up @@ -54,6 +55,13 @@ export interface Line {
title?: string;
}

export interface BroadcastChatHandler {
encodeMsg(msg: string): string;
cleanMsg(msg: string): string;
jumpToMove(msg: string): void;
jumpButton(line: Line): VNode | null;
}

export interface Permissions {
local?: boolean;
broadcast?: boolean;
Expand Down
Loading
Loading