Skip to content

Commit 1794a6d

Browse files
dutowdutow
committed
Lazy load the attachment sidebar
1 parent 0d87411 commit 1794a6d

6 files changed

Lines changed: 81 additions & 27 deletions

File tree

‎app/assets/stylesheets/components/sidebar.css‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,42 @@
6060
margin-bottom: var(--spacing-8);
6161
}
6262

63+
.sidebar-section-wrapper {
64+
margin-bottom: var(--spacing-8);
65+
}
66+
67+
.sidebar .attachments-details summary {
68+
list-style: none;
69+
cursor: pointer;
70+
display: inline-flex;
71+
align-items: center;
72+
gap: var(--spacing-2);
73+
color: var(--color-text-link);
74+
font-size: var(--font-size-sm);
75+
font-weight: var(--font-weight-medium);
76+
padding: var(--spacing-2) var(--spacing-3);
77+
border-radius: var(--border-radius-sm);
78+
transition: background var(--transition-fast), color var(--transition-fast);
79+
}
80+
81+
.sidebar .attachments-details summary::-webkit-details-marker {
82+
display: none;
83+
}
84+
85+
.sidebar .attachments-details summary::before {
86+
content: "▸";
87+
font-size: var(--font-size-xs);
88+
color: var(--color-text-secondary);
89+
}
90+
91+
.sidebar .attachments-details[open] summary::before {
92+
content: "▾";
93+
}
94+
95+
.sidebar .attachments-details summary:hover {
96+
background: var(--color-bg-hover);
97+
}
98+
6399
.sidebar-section.is-search-focus .sidebar-heading {
64100
background: linear-gradient(135deg, var(--color-bg-sidebar-header), var(--color-primary-2));
65101
border-radius: 999px;

‎app/controllers/topics_controller.rb‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
class TopicsController < ApplicationController
2-
before_action :set_topic, only: [ :show, :message_batch, :aware, :read_all, :unread_all, :star, :unstar, :latest_patchset ]
2+
before_action :set_topic, only: [ :show, :message_batch, :attachments_sidebar, :aware, :read_all, :unread_all, :star, :unstar, :latest_patchset ]
33
before_action :require_authentication, only: [ :aware, :aware_bulk, :aware_all, :read_all, :unread_all, :star, :unstar ]
44

55
def index
@@ -57,8 +57,7 @@ def show
5757
.preload(
5858
:sender,
5959
:sender_person,
60-
{ sender_person: :default_alias },
61-
:attachments
60+
{ sender_person: :default_alias }
6261
)
6362

6463
@messages = messages_scope.order(created_at: :asc)
@@ -76,7 +75,9 @@ def show
7675
load_star_state
7776
end
7877

79-
@has_patches = @messages.any? { |msg| msg.attachments.any?(&:patch_extension?) }
78+
@has_patches =
79+
Attachment.joins(:message).where(messages: { topic_id: @topic.id })
80+
.where("attachments.file_name LIKE '%.patch' OR attachments.file_name LIKE '%.diff'").exists?
8081
if @has_patches
8182
latest_message = latest_patchset_message
8283
if latest_message
@@ -106,6 +107,19 @@ def message_batch
106107
render layout: false
107108
end
108109

110+
def attachments_sidebar
111+
message_columns = (Message.column_names - %w[body body_tsv]).map { |c| "messages.#{c}" }
112+
messages = @topic.messages
113+
.select(message_columns)
114+
.preload(:attachments)
115+
.order(created_at: :asc)
116+
117+
@attachment_messages = messages.select { |msg| msg.attachments.any? }
118+
@message_numbers = messages.each_with_index.to_h { |msg, idx| [ msg.id, idx + 1 ] }
119+
120+
render layout: false
121+
end
122+
109123
def aware
110124
last_id = params[:up_to_message_id].presence&.to_i || @topic.messages.maximum(:id)
111125
ThreadAwareness.mark_until(user: current_user, topic: @topic, until_message_id: last_id) if last_id
@@ -362,10 +376,13 @@ def user_state_frame
362376

363377
def latest_patchset_message
364378
@latest_patchset_message ||= @topic.messages
365-
.where(id: Attachment.where(message_id: @topic.messages.select(:id))
379+
.where(id: Attachment.joins(:message)
380+
.where(messages: { topic_id: @topic.id })
381+
.where("attachments.file_name LIKE '%.patch' OR attachments.file_name LIKE '%.diff'")
366382
.select(:message_id))
367383
.order(created_at: :desc)
368-
.find { |msg| msg.attachments.any?(&:patch_extension?) }
384+
.preload(:attachments)
385+
.first
369386
end
370387

371388
def set_topic

‎app/views/topics/_message.html.slim‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,6 @@
4040
- parent_number = (@message_numbers && @message_numbers[message.reply_to.id]) || "-"
4141
.reply-indicator
4242
span.reply-to In reply to: #{message.reply_to.sender_display_alias.name} (##{parent_number})
43-
- if message.attachments.any?
44-
.attachment-indicator
45-
span.attachment-count #{message.attachments.size} attachment(s)
4643
- if message.message_id.present?
4744
.message-archive-link
4845
= link_to "https://postgr.es/m/#{ERB::Util.url_encode(message.message_id)}", target: "_blank", rel: "noopener", data: { turbo: false }, title: "View on postgr.es", "aria-label": "View on postgr.es" do

‎app/views/topics/attachments_sidebar.html.slim‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
= turbo_frame_tag "attachments-sidebar-list" do
2+
- if @attachment_messages.any?
3+
ul.attachments-list
4+
- @attachment_messages.sort_by(&:created_at).reverse_each do |msg|
5+
- number = @message_numbers[msg.id]
6+
li
7+
= link_to "#message-#{msg.id}-attachments", class: "attachment-link" do
8+
span.attachment-target = "##{number}"
9+
span.attachment-date = smart_time_display(msg.created_at)
10+
- names = msg.attachments.map(&:file_name)
11+
- names.first(2).each do |name|
12+
.attachment-name = name
13+
- if names.size > 2
14+
.attachment-more = "+ #{names.size - 2} more"
15+
- else
16+
p No attachments in this thread

‎app/views/topics/show.html.slim‎

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,8 @@
7070
- else
7171
p No participants yet
7272

73-
- attachment_messages = @messages.select { |msg| msg.attachments.any? }
74-
details.sidebar-section open=true
75-
summary.sidebar-heading Attachments
73+
.sidebar-section-wrapper
74+
.sidebar-heading Attachments
7675
.sidebar-section
7776
- if @has_patches
7877
.download-patchset
@@ -84,21 +83,9 @@
8483
span.patchset-added +#{@latest_patchset_stats[:added]}
8584
span.patchset-removed -#{@latest_patchset_stats[:removed]}
8685

87-
- if attachment_messages.any?
88-
ul.attachments-list
89-
- attachment_messages.sort_by(&:created_at).reverse_each do |msg|
90-
- number = @message_numbers[msg.id]
91-
li
92-
= link_to "#message-#{msg.id}-attachments", class: "attachment-link" do
93-
span.attachment-target = "##{number}"
94-
span.attachment-date = smart_time_display(msg.created_at)
95-
- names = msg.attachments.map(&:file_name)
96-
- names.first(2).each do |name|
97-
.attachment-name = name
98-
- if names.size > 2
99-
.attachment-more = "+ #{names.size - 2} more"
100-
- else
101-
p No attachments in this thread
86+
details.attachments-details
87+
summary Show all attachments
88+
= turbo_frame_tag "attachments-sidebar-list", src: attachments_sidebar_topic_path(@topic), loading: :lazy
10289

10390

10491
details.sidebar-section open=true

‎config/routes.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
delete :unstar
7979
get :latest_patchset
8080
get :message_batch
81+
get :attachments_sidebar
8182
end
8283
end
8384
resources :activities, only: [ :index ] do

0 commit comments

Comments
 (0)