Skip to content

Commit

Permalink
Ensure sidebar does not scroll while changing convos (#263)
Browse files Browse the repository at this point in the history
  • Loading branch information
krschacht authored Apr 7, 2024
1 parent c3ddbee commit 0595b4e
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 7 deletions.
6 changes: 6 additions & 0 deletions app/javascript/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,9 @@ document.addEventListener('turbo:before-stream-render', (event) => {
console.log(`${stream.getAttribute('action')} event - ${newTimestamp} ${newTimestamp <= oldTimestamp ? 'REORDER!' : ''}`, stream)
oldTimestamp = newTimestamp
})

console.log('refresh document')
document.addEventListener('turbo:visit', (event) => console.log(`visit ${event.detail.action}`))
document.addEventListener('turbo:morph', () => console.log('morph render'))
document.addEventListener('turbo:frame-render', () => console.log('frame render'))
document.addEventListener('turbo:before-stream-render', () => console.log('stream render'))
6 changes: 4 additions & 2 deletions app/views/conversations/_conversation.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@

<%= link_to conversation.title.presence || "New chat",
conversation_messages_path(conversation),
target: "_top",
class: "flex-1 pl-2 py-2 truncate text-sm text-opacity-1 w-5 radio-behavior",
data: { action: "radio-behavior#select" }
data: {
turbo_frame: "conversation",
action: "radio-behavior#select"
}
%>

<div class="items-center hidden gap-2 pl-2 relationship:flex group-hover:flex">
Expand Down
2 changes: 1 addition & 1 deletion app/views/messages/_main_column.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
<section class="flex flex-col flex-grow overflow-y-auto bg-white dark:bg-gray-800 overflow-x-clip scrollbar-hide" data-controller="scrollable" data-scrollable-not-bottom-class="!block">

<div id="messages" class="relative flex-grow overflow-y-auto overflow-x-clip scrollbar-hide overscroll-contain" data-scrollable-target="scrollable" data-action="scroll->scrollable#scrolled">
<div class="w-full md:max-w-none lg:max-w-[700px] xl:max-w-[810px] mx-auto mt-2">
<div id="messages-container" class="w-full md:max-w-none lg:max-w-[700px] xl:max-w-[810px] mx-auto mt-2">
<%= yield :messages %>
</div>
</div>
Expand Down
6 changes: 3 additions & 3 deletions test/application_system_test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,12 @@ def scroll_to_bottom(selector)
page.execute_script("document.querySelector('#{selector}').scrollTop = document.querySelector('#{selector}').scrollHeight")
end

def assert_did_not_scroll(selector = "section #messages")
def assert_did_not_scroll(selector = "section #messages-container")
raise "No block given" unless block_given?

scroll_position_first_element_relative_viewport = page.evaluate_script("document.querySelector('#{selector}').firstElementChild.getBoundingClientRect().top")
scroll_position_first_element_relative_viewport = page.evaluate_script("document.querySelector('#{selector}').children[1].getBoundingClientRect().top")
yield
new_scroll_position_first_element_relative_viewport = page.evaluate_script("document.querySelector('#{selector}').firstElementChild.getBoundingClientRect().top")
new_scroll_position_first_element_relative_viewport = page.evaluate_script("document.querySelector('#{selector}').children[1].getBoundingClientRect().top")

assert_equal scroll_position_first_element_relative_viewport,
new_scroll_position_first_element_relative_viewport,
Expand Down
2 changes: 1 addition & 1 deletion test/system/conversations/messages_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class ConversationMessagesTest < ApplicationSystemTestCase
test "clicking new compose icon in the top-right starts a new conversation and preserves sidebar scroll" do
click_text @long_conversation.title

assert_did_not_scroll("nav") do
assert_did_not_scroll("#nav-scrollable") do
new_chat = node("new", within: this_conversation)
assert_shows_tooltip new_chat, "New chat"

Expand Down
9 changes: 9 additions & 0 deletions test/system/messages/nav_column_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@ class NavColumnTest < ApplicationSystemTestCase
login_as @user
end

test "clicking conversation in the left column updates the right and preserves scroll position of the left" do
resize_browser_to(1400, 500)
page.execute_script("document.querySelector('#nav-scrollable').scrollTop = 100") # scroll the nav column down slightly

assert_did_not_scroll "#nav-scrollable" do
click_text conversations(:hello_claude).title
end
end

test "clicking conversations in the left side updates the right column, the path, and back button works as expected" do
assistant = @user.assistants.ordered.first

Expand Down

0 comments on commit 0595b4e

Please sign in to comment.