Performance & Aesthetic Hardening: Virtualization, Glassmorphism, and Inter Font#170
Open
jakes1345 wants to merge 2 commits into
Open
Performance & Aesthetic Hardening: Virtualization, Glassmorphism, and Inter Font#170jakes1345 wants to merge 2 commits into
jakes1345 wants to merge 2 commits into
Conversation
added 2 commits
April 16, 2026 09:03
- Implemented react-virtuoso for ChannelMessageList and MemberList to handle large data sets. - Normalized user identity store (globalUsers) for O(1) lookups and consistent state. - Migrated message deduplication to timestamped Map for 24h retention. - Added System Tray support and Autostart plugin for Tauri. - Resolved all TypeScript build errors.
…-animations across layout
Contributor
📝 WalkthroughWalkthroughThis PR modernizes the application with a new Inter font and glass-morphism styling, integrates react-virtuoso for virtualized scrolling of messages and members, refactors user and message deduplication to use timestamped tracking, adds Tauri autostart and tray icon functionality, and simplifies user lookups via a global normalized user store. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/store/handlers/users.ts (1)
241-260:⚠️ Potential issue | 🔴 CriticalRe-read the server after
changeGlobalNick().
state.changeGlobalNick(serverId, oldNick, newNick)updates the store, butserveris still read from the pre-change snapshot. The subsequentsome(user => user.username === newNick)check will miss every channel, so nick-change event messages stop being appended.💡 Proposed fix
const state = store.getState(); state.changeGlobalNick(serverId, oldNick, newNick); - const server = state.servers.find((s) => s.id === serverId); + const updatedState = store.getState(); + const server = updatedState.servers.find((s) => s.id === serverId); if ( server && state.globalSettings.showEvents && state.globalSettings.showNickChanges ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/handlers/users.ts` around lines 241 - 260, The code reads the store into state, calls state.changeGlobalNick(serverId, oldNick, newNick) which mutates the store, but continues using the old snapshot in the local server variable; after changeGlobalNick you must re-read the updated state (call store.getState() again and re-resolve server via state.servers.find(s => s.id === serverId)) and then use that updated server for the subsequent ircClient.getNick, isOurNickChange logic and the server.channels.forEach/user lookup so the newNick is visible to channel.user checks.
🧹 Nitpick comments (5)
src/components/layout/ChannelList.tsx (1)
554-555: Avoid per-rowbackdrop-filterin the scrollable channel list.Applying
glassto every unselected channel means every row gets blur compositing. On long channel lists that's much more expensive to scroll than a flat translucent background, and it works against the PR's performance-hardening goal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/ChannelList.tsx` around lines 554 - 555, The per-row `glass` class (which applies backdrop-filter) is being applied in the ternary that sets each channel row's className (the expression that currently yields either "bg-black/40 text-white shadow-lg shadow-black/20" or `glass ${hoverPrimary}`); move/remove backdrop-filter from individual rows to the scroll container instead and replace per-row `glass` with a lightweight translucent background class (e.g. a flat semi-transparent bg like `bg-white/5` or `bg-black/10`) so rows no longer trigger expensive blur compositing. Update the row-rendering code in ChannelList.tsx (the component/logic that computes the row className and uses `hoverPrimary`) to use the new flat translucent class for the unselected branch and add the `glass`/backdrop-filter style only once on the parent scrollable container.src/store/handlers/messages.ts (1)
417-423: ExtractprocessedMessageIdswrites into one helper.The same
new Map(...); set(id, Date.now())block is now duplicated across every message path. Centralizing it will keep dedup/TTL behavior consistent when this logic changes again.♻️ Possible direction
+function trackProcessedMessageIds( + processed: Map<string, number>, + ids: readonly string[], + now = Date.now(), +): Map<string, number> { + if (ids.length === 0) return processed; + const next = new Map(processed); + for (const id of ids) next.set(id, now); + return next; +}- store.setState((state) => { - const newMap = new Map(state.processedMessageIds); - for (const id of idsToTrack) { - newMap.set(id, Date.now()); - } - return { processedMessageIds: newMap }; - }); + store.setState((state) => ({ + processedMessageIds: trackProcessedMessageIds( + state.processedMessageIds, + idsToTrack, + ), + }));Also applies to: 495-501, 564-570, 706-710, 845-849, 1080-1084, 1239-1243
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/handlers/messages.ts` around lines 417 - 423, There is duplicated logic creating a new Map from state.processedMessageIds and setting ids to the current timestamp; extract this into a single helper (e.g., updateProcessedMessageIds or setProcessedMessageIdsTimestamps) and call it from each place that currently duplicates the block (references: store.setState usages that read/write processedMessageIds and the local variable idsToTrack). The helper should accept the state (or a Map) and an iterable of ids, produce a new Map with the same entries plus each id set to Date.now(), and then each call site should use store.setState(state => ({ processedMessageIds: updateProcessedMessageIds(state, idsToTrack) })) to centralize TTL/dedup behavior.tests/store/multilineDedup.test.ts (1)
90-96: Assert the timestamp payload, not just key presence.These tests still mostly verify
.has(), so they would not catch a regression whereprocessedMessageIdsstops storing meaningful timestamps. Add one assertion on.get(id)being a number and one overwrite/refresh case to cover the new TTL contract.Also applies to: 106-110, 169-175, 205-211, 297-303
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/store/multilineDedup.test.ts` around lines 90 - 96, The test currently only checks presence of keys after calling useStore.setState that updates processedMessageIds; change the assertions to also verify the stored timestamp by calling processedMessageIds.get(id) and asserting it's a number (and within a reasonable range if desired), and add an overwrite/refresh case where you set an id's timestamp, advance time or call the same update again, then assert the new processedMessageIds.get(id) is greater than the prior timestamp to cover the TTL refresh contract; apply the same pattern for the other test blocks that manipulate processedMessageIds (lines referenced in the comment).src/components/layout/ServerList.tsx (1)
74-77: Respect reduced-motion for the new scale animations.These lift/scale effects now run on every primary nav target. Add
motion-reduce:fallbacks so reduced-motion users do not get the hover/tap animation on every interaction.♿ Suggested pattern
- hover:scale-110 active:scale-95 shadow-lg + hover:scale-110 active:scale-95 shadow-lg motion-reduce:transform-none motion-reduce:transition-noneAlso applies to: 213-215, 242-243
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/ServerList.tsx` around lines 74 - 77, Update the hover/tap scale animations in the ServerList component so they respect reduced-motion: wrap the scale utility classes with the Tailwind motion-reduce variants (e.g. change "hover:scale-110" to "motion-reduce:transform-none hover:scale-110" and "active:scale-95" to "motion-reduce:transform-none active:scale-95") wherever those classes appear (including the className block using isSelected/isShimmering and the other two similar blocks referenced in the review); keep the existing conditional classes (bg-discord-primary, glass, shimmer) intact and only add the motion-reduce fallback to prevent scaling for users who prefer reduced motion.src/components/layout/ChannelMessageList.tsx (1)
194-205: Precompute render metadata instead of scanningfilteredMessagesper row.Each rendered item does a full
findIndex()overfilteredMessagesto recover its original position. That turns row rendering into repeated O(n) work and will show up again on large histories despite virtualization. Cache the index /showDate/showHeadermetadata once in auseMemokeyed byfilteredMessages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/ChannelMessageList.tsx` around lines 194 - 205, The current render computes originalIndex via filteredMessages.findIndex and derives showHeader per row (using group.messages[0], previousMessage, timestamps), causing O(n) work per item; replace this by computing an array of metadata in a useMemo keyed on filteredMessages that maps each message.id (or index) to precomputed values: originalIndex, showHeader (apply the same logic comparing adjacent filteredMessages timestamps/type/userId) and any showDate flags; then update the rendering logic to read these fields from the memoized metadata instead of calling findIndex or recomputing comparisons during render (refer to group.messages, filteredMessages, showHeader).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@index.html`:
- Line 11: Replace the external Google Fonts link with locally bundled font
files: add Inter and Roboto Mono font files to the app assets, register them via
`@font-face` in your global CSS (or replace the <link> with a local stylesheet
reference), update the app-wide font-family usage to point to those local font
names, and ensure the build/packaging includes those files (and optionally add
rel="preload" for the primary Inter files) so the app no longer depends on
fonts.googleapis.com/fonts.gstatic.com at runtime.
In `@src-tauri/capabilities/default.json`:
- Around line 13-14: The default capability list currently includes
"autostart:default", granting any renderer the ability to enable
launch-at-login; remove "autostart:default" from the default capabilities array
and instead introduce a dedicated capability (e.g. "autostart:opt-in") for
autostart operations. Update any authorization checks that currently look for
"autostart:default" (or rely on the app-wide default capability) to require the
new dedicated capability name, and ensure the renderer UI only requests/presents
autostart functionality when the new capability is explicitly granted by the
user.
In `@src-tauri/src/lib.rs`:
- Around line 240-270: Replace the unwraps in the tray setup with safe handling:
avoid calling app.default_window_icon().unwrap() in
TrayIconBuilder::new().icon(...) — instead check app.default_window_icon() and
only call .icon(...) when Some(icon) (or use a fallback icon), and change the
on_menu_event closure to mirror the on_tray_icon_event pattern by using if let
Some(window) = app.get_webview_window("main") { let _ = window.show(); let _ =
window.set_focus(); } for the "show" case and let _ = window.hide(); for "hide",
removing all unwrap() calls on get_webview_window, show(), hide(), and
set_focus(); also ensure the "quit" branch still calls app.exit(0) as before.
Finally, consume the --minimized flag passed to the autostart plugin (e.g., read
the startup args or config where autostart is registered and apply the minimized
behavior to the created window) so the flag is not ignored.
In `@src/components/layout/ChannelMessageList.tsx`:
- Around line 107-126: lastScrollTopRef is never updated so getScrollState()
returns a stale value; fix by syncing the ref with Virtuoso scroll events and
applying saved state on mount. Add an onScroll (or use Virtuoso's
atBottomChange/scrollTop callback) to update lastScrollTopRef.current from the
event's scrollTop and update wasAtBottomRef in the same handlers; when the
component mounts and initialScrollState is present, call
virtuosoRef.current.scrollToIndex/scrollTo (or equivalent) to restore the saved
scrollTop/position instead of only toggling followOutput; ensure
getScrollState() reads lastScrollTopRef.current and/or queries virtuosoRef for
current scrollTop so reopened channels resume at the correct position
(references: lastScrollTopRef, getScrollState, virtuosoRef, wasAtBottomRef).
In `@src/components/layout/MemberList.tsx`:
- Around line 536-548: The Virtuoso rows are being keyed by index causing
row-local state to stick to the wrong user when sortedUsers reorders; update the
Virtuoso usage (the Virtuoso component rendering sortedUsers with UserItem) to
provide a computeItemKey that returns user.id (e.g., computeItemKey={(index,
user) => user.id}) so virtual rows are stabilized by user.id (you can keep or
remove the inline key on UserItem, but Virtuoso needs computeItemKey to maintain
correct row-local state).
In `@src/index.css`:
- Around line 28-35: The body rule currently sets background-color: `#0f1113` but
is overridden by the later selector html, body, `#root` { background-color:
`#202225`; }; update to avoid the cascade conflict by either removing
background-color from the body rule (keep background on html, body, `#root`) or
change the later selector to use the intended color (`#0f1113`) so both rules are
consistent; locate the two selectors (body and html, body, `#root`) in
src/index.css and make one of these changes to ensure a single definitive
background color.
- Line 29: The font-family declaration currently quotes Inter; update the
font-family line (the declaration with font-family: 'Inter', -apple-system,
BlinkMacSystemFont, "Segoe UI", Roboto, Helvetica, Arial, sans-serif;) to remove
the unnecessary quotes around Inter so it reads without quotes (leave quoted
names with spaces like "Segoe UI" unchanged) to satisfy the
font-family-name-quotes rule.
In `@src/store/handlers/users.ts`:
- Around line 149-168: The live JOIN handler restores global user info but never
inserts the joiner into the server.channels[].users lists, so new joins don't
appear in per-channel member lists; after building joinedUserMetadata and
calling state.updateGlobalUser (in this block handling a JOIN), locate the
target channel(s) on the found server (server.channels) for the join event and
upsert the user into that channel's users array (create a channel user object
with username, account/realname, metadata: joinedUserMetadata, isOnline: true),
or call the existing helper that updates channel membership if one exists,
ensuring you don't create duplicates and that channel-scoped metadata is set so
later channel updates work.
In `@src/store/index.ts`:
- Around line 1480-1512: changeGlobalNick currently updates state.globalUsers
and channels[].users but misses updating server.privateChats, leaving PM
tabs/selections stale; update changeGlobalNick so when mapping servers for the
matching serverId you also iterate server.privateChats (and any keys/indexes
tied to usernames) to replace occurrences of oldNick with newNick in chat
participants, chat keys, and any stored selection references on the server
object (use symbols: changeGlobalNick, servers, privateChats, channels[].users,
globalUsers) and return the servers array with those privateChats entries
updated alongside the existing channels updates.
---
Outside diff comments:
In `@src/store/handlers/users.ts`:
- Around line 241-260: The code reads the store into state, calls
state.changeGlobalNick(serverId, oldNick, newNick) which mutates the store, but
continues using the old snapshot in the local server variable; after
changeGlobalNick you must re-read the updated state (call store.getState() again
and re-resolve server via state.servers.find(s => s.id === serverId)) and then
use that updated server for the subsequent ircClient.getNick, isOurNickChange
logic and the server.channels.forEach/user lookup so the newNick is visible to
channel.user checks.
---
Nitpick comments:
In `@src/components/layout/ChannelList.tsx`:
- Around line 554-555: The per-row `glass` class (which applies backdrop-filter)
is being applied in the ternary that sets each channel row's className (the
expression that currently yields either "bg-black/40 text-white shadow-lg
shadow-black/20" or `glass ${hoverPrimary}`); move/remove backdrop-filter from
individual rows to the scroll container instead and replace per-row `glass` with
a lightweight translucent background class (e.g. a flat semi-transparent bg like
`bg-white/5` or `bg-black/10`) so rows no longer trigger expensive blur
compositing. Update the row-rendering code in ChannelList.tsx (the
component/logic that computes the row className and uses `hoverPrimary`) to use
the new flat translucent class for the unselected branch and add the
`glass`/backdrop-filter style only once on the parent scrollable container.
In `@src/components/layout/ChannelMessageList.tsx`:
- Around line 194-205: The current render computes originalIndex via
filteredMessages.findIndex and derives showHeader per row (using
group.messages[0], previousMessage, timestamps), causing O(n) work per item;
replace this by computing an array of metadata in a useMemo keyed on
filteredMessages that maps each message.id (or index) to precomputed values:
originalIndex, showHeader (apply the same logic comparing adjacent
filteredMessages timestamps/type/userId) and any showDate flags; then update the
rendering logic to read these fields from the memoized metadata instead of
calling findIndex or recomputing comparisons during render (refer to
group.messages, filteredMessages, showHeader).
In `@src/components/layout/ServerList.tsx`:
- Around line 74-77: Update the hover/tap scale animations in the ServerList
component so they respect reduced-motion: wrap the scale utility classes with
the Tailwind motion-reduce variants (e.g. change "hover:scale-110" to
"motion-reduce:transform-none hover:scale-110" and "active:scale-95" to
"motion-reduce:transform-none active:scale-95") wherever those classes appear
(including the className block using isSelected/isShimmering and the other two
similar blocks referenced in the review); keep the existing conditional classes
(bg-discord-primary, glass, shimmer) intact and only add the motion-reduce
fallback to prevent scaling for users who prefer reduced motion.
In `@src/store/handlers/messages.ts`:
- Around line 417-423: There is duplicated logic creating a new Map from
state.processedMessageIds and setting ids to the current timestamp; extract this
into a single helper (e.g., updateProcessedMessageIds or
setProcessedMessageIdsTimestamps) and call it from each place that currently
duplicates the block (references: store.setState usages that read/write
processedMessageIds and the local variable idsToTrack). The helper should accept
the state (or a Map) and an iterable of ids, produce a new Map with the same
entries plus each id set to Date.now(), and then each call site should use
store.setState(state => ({ processedMessageIds: updateProcessedMessageIds(state,
idsToTrack) })) to centralize TTL/dedup behavior.
In `@tests/store/multilineDedup.test.ts`:
- Around line 90-96: The test currently only checks presence of keys after
calling useStore.setState that updates processedMessageIds; change the
assertions to also verify the stored timestamp by calling
processedMessageIds.get(id) and asserting it's a number (and within a reasonable
range if desired), and add an overwrite/refresh case where you set an id's
timestamp, advance time or call the same update again, then assert the new
processedMessageIds.get(id) is greater than the prior timestamp to cover the TTL
refresh contract; apply the same pattern for the other test blocks that
manipulate processedMessageIds (lines referenced in the comment).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e7fe085-9aa8-4080-9044-317330814663
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
index.htmlpackage.jsonsrc-tauri/Cargo.tomlsrc-tauri/capabilities/default.jsonsrc-tauri/src/lib.rssrc/components/layout/AppLayout.tsxsrc/components/layout/ChannelList.tsxsrc/components/layout/ChannelMessageList.tsxsrc/components/layout/ChatArea.tsxsrc/components/layout/ChatHeader.tsxsrc/components/layout/MemberList.tsxsrc/components/layout/ServerList.tsxsrc/components/message/MessageItem.tsxsrc/index.csssrc/store/handlers/batches.tssrc/store/handlers/messages.tssrc/store/handlers/users.tssrc/store/index.tstailwind.config.jstests/store/multilineDedup.test.ts
|
|
||
| let _tray = TrayIconBuilder::new() | ||
| .tooltip("ObsidianIRC") | ||
| .icon(app.default_window_icon().unwrap().clone()) |
Contributor
There was a problem hiding this comment.
@jakes1345
get_webview_window doesnt exist. Does this build to you?
Compiling ObsidianIRC v0.1.0 (/Users/matheus/projects/ObsidianIRC/src-tauri)
error[E0599]: no method named `get_webview_window` found for reference `&AppHandle<_>` in the current scope
--> src/lib.rs:247:50
|
247 | ... let window = app.get_webview_window("main").unwrap();
| ^^^^^^^^^^^^^^^^^^
|
= help: items from traits can only be used if the trait is in scope
help: there is a method `webview_windows` with a similar name, but with different arguments
--> /Users/matheus/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tauri-2.10.3/src/lib.rs:594:3
|
594 | fn webview_windows(&self) -> HashMap<String, WebviewWindow<R>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: trait `Manager` which provides `get_webview_window` is implemented but not in scope; perhaps you want to import it
|
1 + use tauri::Manager;
ValwareIRC
reviewed
May 4, 2026
Contributor
ValwareIRC
left a comment
ValwareIRC
left a comment
There was a problem hiding this comment.
Thanks for your PR!
May I ask what's the reasoning for TTL pruning with time-expired messages? Currently we prune by how many messages rather than how long they've been there. Any particular benefit?
| mtags.msgid, | ||
| ]); | ||
| const newMap = new Map(state.processedMessageIds); | ||
| newMap.set(mtags.msgid, Date.now()); |
Contributor
There was a problem hiding this comment.
it seems like you are using Date.now() for TTL pruning for deleting old messages from the UI, but we do that by count not by timestamp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This PR introduces significant performance optimizations and a modern UI/UX overhaul to the ObsidianIRC client.
Key Changes
react-virtuosoto handle thousands of users smoothly without DOM-induced lag.These changes collectively modernize the obsidian aesthetic while hardening the engine for high-traffic environments.
Summary by CodeRabbit
Release Notes
New Features
Style