Skip to content

Conversation

tkhumush
Copy link

Display counts next to the reply and repost icons in the note action bar. The counts are queried from nostrdb and only shown when greater than zero. Spacing is minimal (2px) between icons and counts for a clean appearance.

Display counts next to the reply and repost icons in the note action bar.
The counts are queried from nostrdb and only shown when greater than zero.
Spacing is minimal (2px) between icons and counts for a clean appearance.
@alltheseas alltheseas requested a review from Copilot October 19, 2025 15:34
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds reply and repost count displays to the note action bar. The counts are fetched from nostrdb and displayed next to their respective icons when greater than zero.

Key Changes:

  • Added count_note_replies and count_note_reposts functions to query nostrdb for counts
  • Modified reply_button and quote_repost_button to display counts with minimal spacing
  • Updated render_note_actionbar signature to accept and pass count parameters

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +456 to +457
let reply_count = count_note_replies(self.note_context.ndb, txn, self.note.id());
let repost_count = count_note_reposts(self.note_context.ndb, txn, self.note.id());
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

These queries are executed for every note rendered. Consider caching the counts or batching queries to avoid repeated database lookups, especially when rendering large lists of notes.

Copilot uses AI. Check for mistakes.

Comment on lines +864 to +866
ndb.query(txn, &[filter], 500)
.map(|results| results.len())
.unwrap_or(0)
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

The query fetches up to 500 results just to count them. Use a count-specific query method if available in nostrdb, or limit the results to a smaller number since only the count is needed.

Copilot uses AI. Check for mistakes.

Comment on lines +876 to +878
ndb.query(txn, &[filter], 500)
.map(|results| results.len())
.unwrap_or(0)
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

The query fetches up to 500 results just to count them. Use a count-specific query method if available in nostrdb, or limit the results to a smaller number since only the count is needed.

Copilot uses AI. Check for mistakes.

Copy link
Member

Choose a reason for hiding this comment

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

This queries every frame and even with small queries I've seen it's not performant to do this. You may have seen the unseen notification indicator impl, when ndb::query occurs it leads to >5ms many times


if reply_count > 0 {
ui.add_space(2.0);
ui.label(egui::RichText::new(format!("{}", reply_count)).size(13.0));
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

The magic number 13.0 for font size is duplicated in both reply and repost buttons. Consider extracting this to a named constant for consistency and easier maintenance.

Copilot uses AI. Check for mistakes.

let combined_resp = resp.union(put_resp);

if reply_count > 0 {
ui.add_space(2.0);
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

The magic number 2.0 for spacing is duplicated in both reply and repost buttons. Consider extracting this to a named constant for consistency and easier maintenance.

Copilot uses AI. Check for mistakes.

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.

2 participants