sitting33: 2 Don't mark messages as read when window is not in focus Show number of unread buffers or messages in title 4 files changed, 171 insertions(+), 37 deletions(-)
Oops, I forgot to add that this patch is for gamja. Sorry!
В письме от пятница, 7 июля 2023 г. 15:55:08 +03 пользователь Simon Ser написал:
this.handleSettingsChange.bind(this);
this.handleSwitchSubmit.bind(this);
this.handleDocumentFocus.bind(this);
server:
getReceipt(stored,
bottom
server:
getReceipt(stored,
buf.messages[buf.messages.length - 1];
server.users.has(buf.name)) {
buf.server);
this.baseTitle;
receiptFromMessage(msg) };
document.hasFocus()) {
receiptFromMessage(msg);
this.handleDocumentFocus);
this.clients.get(buffer.server);
buffer.name, server:
getReceipt(stored,
bottom until we reach
lastReadReceipt)) {
trigger a buffer to be marked as
"INVITE"].includes(msg.command)) {
1);
by server
history
"chathistory");
networks") {
have a URL to
the user to
of the new
*after* the
we have a URL to
it, ask the user to
&&
readReceipt },
props.settings.secondsInTimestamps;
{this.state.titleCounter === "none"}
{this.state.titleCounter === "buffers"}
{this.state.titleCounter === "messages"}
title
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~emersion/public-inbox/patches/42117/mbox | git am -3Learn more about email & git
--- I split the previous revision of the patch into two parts. The one defers sending MARKREAD when the tab isn't focused. When it regains focus, the currently active buffers gets marked as read.
Thanks, the meat of the patch looks good to me :)
I had to extract a new method, markBufferAsRead, that is called in switchBuffer and handleDocumentFocus. handleDocumentFocus calls it with updatePrevReceipt = false, so that the divider in between read and unread messages doesn't disappear when we focus the window twice.
Can we call markBufferAsRead() from the MARKREAD handler? (line 1153)The MARKREAD handler also counts the number of messages still unread, but markBufferAsRead() simply marks everything as read. MARKREAD could still be useful for more sophisticated clients that know exactly which messages the user has read.Can we never update prevReadReceipt in markBufferAsRead(), and instead always update it in switchBuffer()?
components/app.js | 77 +++++++++++++++++++++++++++++------------------ 1 file changed, 48 insertions(+), 29 deletions(-) diff --git a/components/app.js b/components/app.js index 3987749..5575527 100644 --- a/components/app.js +++ b/components/app.js @@ -228,6 +228,7 @@ export default class App extends Component { this.handleSettingsChange = this.handleSettingsChange.bind(this); this.handleSettingsDisconnect = this.handleSettingsDisconnect.bind(this); this.handleSwitchSubmit = this.handleSwitchSubmit.bind(this); + this.handleDocumentFocus = this.handleDocumentFocus.bind(this); this.state.settings = { ...this.state.settings, @@ -528,33 +529,13 @@ export default class App extends Component { client.setReadMarker(storedBuffer.name, readReceipt.time); } - switchBuffer(id) { - let buf; - this.setState((state) => { - buf = State.getBuffer(state, id); + markBufferAsRead(id, updatePrevReceipt) { + this.setState(state => { + const buf = State.getBuffer(state, id); if (!buf) { return; } - let client = this.clients.get(buf.server); - let stored = this.bufferStore.get({ name: buf.name, server: client.params }); - let prevReadReceipt = getReceipt(stored, ReceiptType.READ); - // TODO: only mark as read if user scrolled at the bottom
This TODO got lost, but is still relevant AFAIK.
- let update = State.updateBuffer(state, buf.id, { - unread: Unread.NONE, - prevReadReceipt, - }); - - return { ...update, activeBuffer: buf.id }; - }, () => { - if (!buf) { - return; - } - - if (this.buffer.current) { - this.buffer.current.focus(); - } - let client = this.clients.get(buf.server); for (let notif of this.messageNotifications) { @@ -563,6 +544,9 @@ export default class App extends Component { } } + let stored = this.bufferStore.get({ name: buf.name, server: client.params }); + let prevReadReceipt = getReceipt(stored, ReceiptType.READ); + if (buf.messages.length > 0) { let lastMsg = buf.messages[buf.messages.length - 1]; let stored = { @@ -576,6 +560,37 @@ export default class App extends Component { } } + if (updatePrevReceipt) { + return State.updateBuffer(state, buf.id, { + unread: Unread.NONE, + prevReadReceipt, + }); + } else { + return State.updateBuffer(state, buf.id, { + unread: Unread.NONE, + }); + } + }); + } + + switchBuffer(id) { + let buf; + this.setState((state) => { + buf = State.getBuffer(state, id); + if (!buf) { + return; + } + + return { activeBuffer: buf.id }; + }, () => { + if (!buf) { + return; + } + + if (this.buffer.current) { + this.buffer.current.focus(); + } + let server = this.state.servers.get(buf.server); if (buf.type === BufferType.NICK && !server.users.has(buf.name)) { this.whoUserBuffer(buf.name, buf.server); @@ -585,11 +600,7 @@ export default class App extends Component { this.whoChannelBuffer(buf.name, buf.server); } - if (buf.type !== BufferType.SERVER) { - document.title = buf.name + ' · ' + this.baseTitle; - } else { - document.title = this.baseTitle; - }
We still need to update the webpage title here.
+ this.markBufferAsRead(buf.id, true); }); } @@ -717,7 +728,7 @@ export default class App extends Component { let prevReadReceipt = buf.prevReadReceipt; let receipts = { [ReceiptType.DELIVERED]: receiptFromMessage(msg) }; - if (this.state.activeBuffer !== buf.id) { + if (this.state.activeBuffer !== buf.id || !document.hasFocus()) { unread = Unread.union(unread, msgUnread); } else { receipts[ReceiptType.READ] = receiptFromMessage(msg); @@ -1912,13 +1923,21 @@ export default class App extends Component { } } + handleDocumentFocus() { + this.markBufferAsRead(this.state.activeBuffer, false); + } + componentDidMount() { this.baseTitle = document.title; setupKeybindings(this); + + window.addEventListener("focus", this.handleDocumentFocus); } componentWillUnmount() { document.title = this.baseTitle; + + window.removeEventListener("blur", this.handleDocumentFocus);
Typo: "focus"
} render() { -- 2.41.0
--- This adds the number of unread buffers or messages (configurable) into the document title. components/app.js | 98 ++++++++++++++++++++++++++++++++++--- components/settings-form.js | 32 ++++++++++++ state.js | 1 + 3 files changed, 123 insertions(+), 8 deletions(-) diff --git a/components/app.js b/components/app.js index 5575527..ab174ce 100644 --- a/components/app.js +++ b/components/app.js @@ -570,6 +570,8 @@ export default class App extends Component { unread: Unread.NONE, }); } + }, () => { + this.updateDocumentTitle(); }); } @@ -604,6 +606,63 @@ export default class App extends Component { }); } + updateDocumentTitle() { + const currentBuffer = State.getBuffer(this.state, this.state.activeBuffer); + const currentServerName = getServerName(this.state.servers.get(currentBuffer.server)); + + let resultingTitle = ""; + + // add unread counter + let unreadCounter = 0; + if (this.state.settings.titleCounter === "buffers") { + for (const buffer of this.state.buffers.values()) { + if (buffer.unread > Unread.NONE) {
unread is a string-based enum, can't be compared this way. But we can do this
instead:
Unread.compare(buffer.unread, Unread.NONE) > 0
But buffer.unread !== Unread.NONE is probably simpler here.
+ unreadCounter += 1;
Nit: JS has ++ to increment
+ } + } + } else if (this.state.settings.titleCounter === "messages") { + for (const buffer of this.state.buffers.values()) { + if (buffer.unread === Unread.NONE) { + continue; + } + + const client = this.clients.get(buffer.server); + const stored = this.bufferStore.get({name: buffer.name, server: client.params}); + const lastReadReceipt = getReceipt(stored, ReceiptType.READ); + + // go through every message starting at the bottom until we reach + // a message that's already been read + let bufferUnreadCounter = 0; + let msgIndex = buffer.messages.length - 1; + let msg = buffer.messages[msgIndex]; + while (msg && !isMessageBeforeReceipt(msg, lastReadReceipt)) { + // these are messages that can trigger a buffer to be marked as unread + if (["PRIVMSG", "NOTICE", "INVITE"].includes(msg.command)) { + bufferUnreadCounter++; + } + msg = buffer.messages[--msgIndex]; + } + + unreadCounter += Math.max(bufferUnreadCounter, 1);
Hm, the number of messages doesn't seem super useful to me. What I suggested in the previous version was to only count highlights. Basically the same as the titleCounter === "buffers" case above but with buffer.unread === Unread.HIGHLIGHT.
+ } + } + if (unreadCounter > 0) { + resultingTitle += `(${unreadCounter}) `; + } + + // add current buffer name + if (currentBuffer.type !== BufferType.SERVER) { + resultingTitle += currentBuffer.name + ' · '; + } + + resultingTitle += currentServerName + ' · '; + + // add the base title (document.title) + resultingTitle += this.baseTitle; + + document.title = resultingTitle; + } + prepareChatMessage(serverID, msg) { // Treat server-wide broadcasts as highlights. They're sent by server // operators and can contain important information. @@ -749,6 +808,12 @@ export default class App extends Component { this.sendReadReceipt(client, stored); } return { unread, prevReadReceipt }; + }, () => { + // don't update the title if we're getting the history + const chatHistoryBatch = irc.findBatchByType(msg, "chathistory"); + if (!chatHistoryBatch) {
Style nit: the intermediary variable isn't really necessary here
+ this.updateDocumentTitle(); + } }); } @@ -1148,17 +1213,30 @@ export default class App extends Component { } let name = msg.params[0].slice(1); let batch = client.batches.get(name); - if (!batch || batch.type !== "soju.im/bouncer-networks") { + if (!batch) { break; } - // We've received a BOUNCER NETWORK batch. If we have a URL to - // auto-open and no existing network matches it, ask the user to - // create a new network. - if (this.autoOpenURL && this.autoOpenURL.host && !this.findBouncerNetIDByHost(this.autoOpenURL.host)) { - this.openURL(this.autoOpenURL); - this.autoOpenURL = null; + switch (batch.type) { + case "chathistory": + // Since the component hasn't updated because of the new + // messages yet, we need to update the title *after* the + // new state has been applied. + this.forceUpdate(() => { + this.updateDocumentTitle(); + });
Hm, forceUpdate() doesn't sound great. Maybe we can setState({}, () => { … })
instead?
+ break; + case "soju.im/bouncer-networks": + // We've received a BOUNCER NETWORK batch. If we have a URL to + // auto-open and no existing network matches it, ask the user to + // create a new network. + if (this.autoOpenURL && this.autoOpenURL.host && !this.findBouncerNetIDByHost(this.autoOpenURL.host)) { + this.openURL(this.autoOpenURL); + this.autoOpenURL = null; + } + break; } + break; case "MARKREAD": target = msg.params[0]; @@ -1212,6 +1290,8 @@ export default class App extends Component { closed, receipts: { [ReceiptType.READ]: readReceipt }, }); + + this.updateDocumentTitle(); }); break; default: @@ -1908,7 +1988,9 @@ export default class App extends Component { handleSettingsChange(settings) { store.settings.put(settings); - this.setState({ settings }); + this.setState({ settings }, () => { + this.updateDocumentTitle(); + }); } handleSettingsDisconnect() { diff --git a/components/settings-form.js b/components/settings-form.js index 31e045e..482f5b4 100644 --- a/components/settings-form.js +++ b/components/settings-form.js @@ -8,6 +8,7 @@ export default class SettingsForm extends Component { this.state.secondsInTimestamps = props.settings.secondsInTimestamps; this.state.bufferEvents = props.settings.bufferEvents; + this.state.titleCounter = props.settings.titleCounter; this.handleInput = this.handleInput.bind(this); this.handleSubmit = this.handleSubmit.bind(this); @@ -67,6 +68,37 @@ export default class SettingsForm extends Component { </label> <br/><br/> + <label> + <input + type="radio" + name="titleCounter" + value="none" + checked=${this.state.titleCounter === "none"} + /> + Don't show unread counter in title
Maybe we could be a bit more explicit here, e.g. "in page title".
+ </label> + <br/> + <label> + <input + type="radio" + name="titleCounter" + value="buffers" + checked=${this.state.titleCounter === "buffers"} + /> + Show number of unread buffers in title + </label> + <br/> + <label> + <input + type="radio" + name="titleCounter" + value="messages" + checked=${this.state.titleCounter === "messages"} + /> + Show number of unread messages in title + </label> + <br/><br/> + <label> <input type="radio" diff --git a/state.js b/state.js index 1c1bc5a..13b1b5a 100644 --- a/state.js +++ b/state.js @@ -213,6 +213,7 @@ export const State = { settings: { secondsInTimestamps: true, bufferEvents: BufferEventsDisplayMode.FOLD, + titleCounter: "buffers", }, }; }, -- 2.41.0
Oops, I forgot to add that this patch is for gamja. Sorry!