From 8d10241e6b8da25fc556e619c74f780cac042890 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Fri, 11 Jul 2025 15:31:43 +0100 Subject: [PATCH 01/24] Add Pagy dependency --- Gemfile | 2 ++ Gemfile.lock | 3 +++ 2 files changed, 5 insertions(+) diff --git a/Gemfile b/Gemfile index ee24063f9eeb..d0e7961c366b 100644 --- a/Gemfile +++ b/Gemfile @@ -420,3 +420,5 @@ end gem "openproject-octicons", "~>19.29.0" gem "openproject-octicons_helper", "~>19.29.0" gem "openproject-primer_view_components", "~>0.73.1" + +gem "pagy", "~> 9.3" diff --git a/Gemfile.lock b/Gemfile.lock index 9fc37b0829b9..7f307e058208 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1119,6 +1119,7 @@ GEM ostruct (0.6.3) ox (2.14.23) bigdecimal (>= 3.0) + pagy (9.3.5) paper_trail (16.0.0) activerecord (>= 6.1) request_store (~> 1.4) @@ -1710,6 +1711,7 @@ DEPENDENCIES opentelemetry-sdk (~> 1.9) overviews! ox + pagy (~> 9.3) paper_trail (~> 16.0.0) parallel_tests (~> 4.0) pdf-inspector (~> 1.2) @@ -2147,6 +2149,7 @@ CHECKSUMS ostruct (0.6.3) sha256=95a2ed4a4bd1d190784e666b47b2d3f078e4a9efda2fccf18f84ddc6538ed912 overviews (1.0.0) ox (2.14.23) sha256=4a9aedb4d6c78c5ebac1d7287dc7cc6808e14a8831d7adb727438f6a1b461b66 + pagy (9.3.5) sha256=78a9513150b96f872c092ab1cd95bb818ea29b2c417a4302290bc9293f8f0fd7 paper_trail (16.0.0) sha256=e9b9f0fb1b8b590c8231cfa931b282ba92f90e066e393930a5e1c61ae4c5019d parallel (1.27.0) sha256=4ac151e1806b755fb4e2dc2332cbf0e54f2e24ba821ff2d3dcf86bf6dc4ae130 parallel_tests (4.10.1) sha256=df05458c691462b210f7a41fc2651d4e4e8a881e8190e6d1e122c92c07735d70 From 874bdebeb7773b960107e3560c0ca9994ac973a3 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Fri, 11 Jul 2025 15:46:09 +0100 Subject: [PATCH 02/24] initializer --- config/initializers/pagy.rb | 240 ++++++++++++++++++++++++++++++++++++ 1 file changed, 240 insertions(+) create mode 100644 config/initializers/pagy.rb diff --git a/config/initializers/pagy.rb b/config/initializers/pagy.rb new file mode 100644 index 000000000000..a21dc471b3e9 --- /dev/null +++ b/config/initializers/pagy.rb @@ -0,0 +1,240 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +# Pagy initializer file (9.3.3) +# Customize only what you really need and notice that the core Pagy works also without any of the following lines. +# Should you just cherry pick part of this file, please maintain the require-order of the extras + +# Pagy Variables +# See https://ddnexus.github.io/pagy/docs/api/pagy#variables +# You can set any pagy variable as a Pagy::DEFAULT. They can also be overridden per instance by just passing them to +# Pagy.new|Pagy::Countless.new|Pagy::Calendar::*.new or any of the #pagy* controller methods +# Here are the few that make more sense as DEFAULTs: +# Pagy::DEFAULT[:limit] = 20 # default +# Pagy::DEFAULT[:size] = 7 # default +# Pagy::DEFAULT[:ends] = true # default +# Pagy::DEFAULT[:page_param] = :page # default +# Pagy::DEFAULT[:count_args] = [] # example for non AR ORMs +# Pagy::DEFAULT[:max_pages] = 3000 # example + +# Extras +# See https://ddnexus.github.io/pagy/categories/extra + +# Legacy Compatibility Extras + +# Size extra: Enable the Array type for the `:size` variable (e.g. `size: [1,4,4,1]`) +# See https://ddnexus.github.io/pagy/docs/extras/size +# require 'pagy/extras/size' # must be required before the other extras + +# Backend Extras + +# Arel extra: For better performance utilizing grouped ActiveRecord collections: +# See: https://ddnexus.github.io/pagy/docs/extras/arel +# require 'pagy/extras/arel' + +# Array extra: Paginate arrays efficiently, avoiding expensive array-wrapping and without overriding +# See https://ddnexus.github.io/pagy/docs/extras/array +# require 'pagy/extras/array' + +# Calendar extra: Add pagination filtering by calendar time unit (year, quarter, month, week, day) +# See https://ddnexus.github.io/pagy/docs/extras/calendar +# require 'pagy/extras/calendar' +# Default for each calendar unit class in IRB: +# >> Pagy::Calendar::Year::DEFAULT +# >> Pagy::Calendar::Quarter::DEFAULT +# >> Pagy::Calendar::Month::DEFAULT +# >> Pagy::Calendar::Week::DEFAULT +# >> Pagy::Calendar::Day::DEFAULT +# +# Uncomment the following lines, if you need calendar localization without using the I18n extra +# module LocalizePagyCalendar +# def localize(time, opts) +# ::I18n.l(time, **opts) +# end +# end +# Pagy::Calendar.prepend LocalizePagyCalendar + +# Countless extra: Paginate without any count, saving one query per rendering +# See https://ddnexus.github.io/pagy/docs/extras/countless +# require 'pagy/extras/countless' +# Pagy::DEFAULT[:countless_minimal] = false # default (eager loading) + +# Elasticsearch Rails extra: Paginate `ElasticsearchRails::Results` objects +# See https://ddnexus.github.io/pagy/docs/extras/elasticsearch_rails +# Default :pagy_search method: change only if you use also +# the searchkick or meilisearch extra that defines the same +# Pagy::DEFAULT[:elasticsearch_rails_pagy_search] = :pagy_search +# Default original :search method called internally to do the actual search +# Pagy::DEFAULT[:elasticsearch_rails_search] = :search +# require 'pagy/extras/elasticsearch_rails' + +# Headers extra: http response headers (and other helpers) useful for API pagination +# See https://ddnexus.github.io/pagy/docs/extras/headers +# require 'pagy/extras/headers' +# Pagy::DEFAULT[:headers] = { page: 'Current-Page', +# limit: 'Page-Items', +# count: 'Total-Count', +# pages: 'Total-Pages' } # default + +# Keyset extra: Paginate with the Pagy keyset pagination technique +# See https://ddnexus.github.io/pagy/docs/extras/keyset +# require 'pagy/extras/keyset' + +# Meilisearch extra: Paginate `Meilisearch` result objects +# See https://ddnexus.github.io/pagy/docs/extras/meilisearch +# Default :pagy_search method: change only if you use also +# the elasticsearch_rails or searchkick extra that define the same method +# Pagy::DEFAULT[:meilisearch_pagy_search] = :pagy_search +# Default original :search method called internally to do the actual search +# Pagy::DEFAULT[:meilisearch_search] = :ms_search +# require 'pagy/extras/meilisearch' + +# Metadata extra: Provides the pagination metadata to Javascript frameworks like Vue.js, react.js, etc. +# See https://ddnexus.github.io/pagy/docs/extras/metadata +# you must require the JS Tools internal extra (BEFORE the metadata extra) ONLY if you need also the :sequels +# require 'pagy/extras/js_tools' +# require 'pagy/extras/metadata' +# For performance reasons, you should explicitly set ONLY the metadata you use in the frontend +# Pagy::DEFAULT[:metadata] = %i[scaffold_url page prev next last] # example + +# Searchkick extra: Paginate `Searchkick::Results` objects +# See https://ddnexus.github.io/pagy/docs/extras/searchkick +# Default :pagy_search method: change only if you use also +# the elasticsearch_rails or meilisearch extra that defines the same +# Pagy::DEFAULT[:searchkick_pagy_search] = :pagy_search +# Default original :search method called internally to do the actual search +# Pagy::DEFAULT[:searchkick_search] = :search +# require 'pagy/extras/searchkick' +# uncomment if you are going to use Searchkick.pagy_search +# Searchkick.extend Pagy::Searchkick + +# Frontend Extras + +# Bootstrap extra: Add nav, nav_js and combo_nav_js helpers and templates for Bootstrap pagination +# See https://ddnexus.github.io/pagy/docs/extras/bootstrap +# require 'pagy/extras/bootstrap' + +# Bulma extra: Add nav, nav_js and combo_nav_js helpers and templates for Bulma pagination +# See https://ddnexus.github.io/pagy/docs/extras/bulma +# require 'pagy/extras/bulma' + +# Pagy extra: Add the pagy styled versions of the javascript-powered navs +# and a few other components to the Pagy::Frontend module. +# See https://ddnexus.github.io/pagy/docs/extras/pagy +# require 'pagy/extras/pagy' + +# Multi size var used by the *_nav_js helpers +# See https://ddnexus.github.io/pagy/docs/extras/pagy#steps +# Pagy::DEFAULT[:steps] = { 0 => 5, 540 => 7, 720 => 9 } # example + +# Feature Extras + +# Gearbox extra: Automatically change the limit per page depending on the page number +# See https://ddnexus.github.io/pagy/docs/extras/gearbox +# require 'pagy/extras/gearbox' +# set to false only if you want to make :gearbox_extra an opt-in variable +# Pagy::DEFAULT[:gearbox_extra] = false # default true +# Pagy::DEFAULT[:gearbox_limit] = [15, 30, 60, 100] # default + +# Limit extra: Allow the client to request a custom limit per page with an optional selector UI +# See https://ddnexus.github.io/pagy/docs/extras/limit +# require 'pagy/extras/limit' +# set to false only if you want to make :limit_extra an opt-in variable +# Pagy::DEFAULT[:limit_extra] = false # default true +# Pagy::DEFAULT[:limit_param] = :limit # default +# Pagy::DEFAULT[:limit_max] = 100 # default + +# Overflow extra: Allow for easy handling of overflowing pages +# See https://ddnexus.github.io/pagy/docs/extras/overflow +# require 'pagy/extras/overflow' +# Pagy::DEFAULT[:overflow] = :empty_page # default (other options: :last_page and :exception) + +# Trim extra: Remove the page=1 param from links +# See https://ddnexus.github.io/pagy/docs/extras/trim +# require 'pagy/extras/trim' +# set to false only if you want to make :trim_extra an opt-in variable +# Pagy::DEFAULT[:trim_extra] = false # default true + +# Standalone extra: Use pagy in non Rack environment/gem +# See https://ddnexus.github.io/pagy/docs/extras/standalone +# require 'pagy/extras/standalone' +# Pagy::DEFAULT[:url] = 'http://www.example.com/subdir' # optional default + +# Jsonapi extra: Implements JSON:API specifications +# See https://ddnexus.github.io/pagy/docs/extras/jsonapi +# require 'pagy/extras/jsonapi' # must be required after the other extras +# set to false only if you want to make :jsonapi an opt-in variable +# Pagy::DEFAULT[:jsonapi] = false # default true + +# Rails +# Enable the .js file required by the helpers that use javascript +# (pagy*_nav_js, pagy*_combo_nav_js, and pagy_limit_selector_js) +# See https://ddnexus.github.io/pagy/docs/api/javascript + +# With the asset pipeline +# Sprockets need to look into the pagy javascripts dir, so add it to the assets paths +# Rails.application.config.assets.paths << Pagy.root.join('javascripts') + +# I18n + +# Pagy internal I18n: ~18x faster using ~10x less memory than the i18n gem +# See https://ddnexus.github.io/pagy/docs/api/i18n +# Notice: No need to configure anything in this section if your app uses only "en" +# or if you use the i18n extra below +# +# Examples: +# load the "de" built-in locale: +# Pagy::I18n.load(locale: 'de') +# +# load the "de" locale defined in the custom file at :filepath: +# Pagy::I18n.load(locale: 'de', filepath: 'path/to/pagy-de.yml') +# +# load the "de", "en" and "es" built-in locales: +# (the first passed :locale will be used also as the default_locale) +# Pagy::I18n.load({ locale: 'de' }, +# { locale: 'en' }, +# { locale: 'es' }) +# +# load the "en" built-in locale, a custom "es" locale, +# and a totally custom locale complete with a custom :pluralize proc: +# (the first passed :locale will be used also as the default_locale) +# Pagy::I18n.load({ locale: 'en' }, +# { locale: 'es', filepath: 'path/to/pagy-es.yml' }, +# { locale: 'xyz', # not built-in +# filepath: 'path/to/pagy-xyz.yml', +# pluralize: lambda{ |count| ... } ) + +# I18n extra: uses the standard i18n gem which is ~18x slower using ~10x more memory +# than the default pagy internal i18n (see above) +# See https://ddnexus.github.io/pagy/docs/extras/i18n +# require 'pagy/extras/i18n' + +# When you are done setting your own default freeze it, so it will not get changed accidentally +Pagy::DEFAULT.freeze From 2468d4d98a08bba064756eedb9836bc2ce86fffc Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 18 Sep 2025 19:10:23 +0300 Subject: [PATCH 03/24] Set up POC for infinite scrolling on AT (Messy Commit!) --- .../activities_tab/index_component.html.erb | 65 ++++------ .../activities_tab/index_component.rb | 22 +++- .../journals/index_component.html.erb | 88 ++++--------- .../journals/index_component.rb | 81 +++--------- .../infinite_scroll_component.html.erb | 25 ++++ .../journals/infinite_scroll_component.rb | 57 +++++++++ .../journals/page_component.html.erb | 20 +++ .../activities_tab/journals/page_component.rb | 62 +++++++++ .../activities_tab/shared_helpers.rb | 8 ++ .../concerns/op_turbo/component_stream.rb | 17 +++ .../activities_tab_controller.rb | 119 +++++++++++++----- config/initializers/pagy.rb | 4 +- config/initializers/permissions.rb | 2 +- config/routes.rb | 1 + 14 files changed, 368 insertions(+), 203 deletions(-) create mode 100644 app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb create mode 100644 app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb create mode 100644 app/components/work_packages/activities_tab/journals/page_component.html.erb create mode 100644 app/components/work_packages/activities_tab/journals/page_component.rb diff --git a/app/components/work_packages/activities_tab/index_component.html.erb b/app/components/work_packages/activities_tab/index_component.html.erb index fcacece9d01f..73c5feb176b4 100644 --- a/app/components/work_packages/activities_tab/index_component.html.erb +++ b/app/components/work_packages/activities_tab/index_component.html.erb @@ -1,50 +1,33 @@ <%= - if deferred - render( - WorkPackages::ActivitiesTab::Journals::IndexComponent.new(work_package:, filter:, deferred:) - ) - else - component_wrapper(tag: "turbo-frame") do - flex_layout(classes: "work-packages-activities-tab-index-component") do |activities_tab_wrapper_container| - activities_tab_wrapper_container.with_row(classes: "work-packages-activities-tab-index-component--errors") do - render( - WorkPackages::ActivitiesTab::ErrorStreamComponent.new - ) - end + component_wrapper(tag: "turbo-frame") do + flex_layout(classes: "work-packages-activities-tab-index-component") do |activities_tab_wrapper_container| + activities_tab_wrapper_container.with_row(classes: "work-packages-activities-tab-index-component--errors") do + render(error_stream_component) + end + + activities_tab_wrapper_container.with_row(id: index_content_wrapper_key, data: wrapper_data_attributes) do + flex_layout do |activities_tab_container| + activities_tab_container.with_row(mb: 2) do + render(filter_and_sorting_component) + end - activities_tab_wrapper_container.with_row(id: index_content_wrapper_key, data: wrapper_data_attributes) do - flex_layout do |activities_tab_container| - activities_tab_container.with_row(mb: 2) do - render( - WorkPackages::ActivitiesTab::Journals::FilterAndSortingComponent.new( - work_package:, - filter: - ) - ) + activities_tab_container.with_row(flex_layout: true, classes: "work-packages-activities-tab-index-component--content-container", mt: 3) do |journals_wrapper_container| + journals_wrapper_container.with_row( + classes: "work-packages-activities-tab-index-component--journals-container work-packages-activities-tab-index-component--journals-container_with-initial-input-compensation", + data: { "work-packages--activities-tab--index-target": "journalsContainer" } + ) do + render(list_journals_component) end - activities_tab_container.with_row(flex_layout: true, classes: "work-packages-activities-tab-index-component--content-container", mt: 3) do |journals_wrapper_container| + if adding_comment_allowed? journals_wrapper_container.with_row( - classes: "work-packages-activities-tab-index-component--journals-container work-packages-activities-tab-index-component--journals-container_with-initial-input-compensation", - data: { "work-packages--activities-tab--index-target": "journalsContainer" } + id: add_comment_wrapper_key, + classes: "work-packages-activities-tab-index-component--input-container work-packages-activities-tab-index-component--input-container_sort-#{journal_sorting}", + p: 3, + bg: :subtle, + data: add_comment_wrapper_data_attributes ) do - render( - WorkPackages::ActivitiesTab::Journals::IndexComponent.new(work_package:, filter:) - ) - end - - if adding_comment_allowed? - journals_wrapper_container.with_row( - id: add_comment_wrapper_key, - classes: "work-packages-activities-tab-index-component--input-container work-packages-activities-tab-index-component--input-container_sort-#{journal_sorting}", - p: 3, - bg: :subtle, - data: add_comment_wrapper_data_attributes - ) do - render( - WorkPackages::ActivitiesTab::Journals::NewComponent.new(work_package:, filter:, last_server_timestamp:) - ) - end + render(add_journal_component) end end end diff --git a/app/components/work_packages/activities_tab/index_component.rb b/app/components/work_packages/activities_tab/index_component.rb index c1bd41061ef1..574870697f5c 100644 --- a/app/components/work_packages/activities_tab/index_component.rb +++ b/app/components/work_packages/activities_tab/index_component.rb @@ -37,13 +37,13 @@ class IndexComponent < ApplicationComponent include WorkPackages::ActivitiesTab::SharedHelpers include WorkPackages::ActivitiesTab::StimulusControllers - def initialize(work_package:, last_server_timestamp:, filter: :all, deferred: false) + def initialize(work_package:, journals:, last_server_timestamp:, filter: :all) super @work_package = work_package + @journals = journals @filter = filter @last_server_timestamp = last_server_timestamp - @deferred = deferred end def self.wrapper_key = "work-package-activities-tab-content" @@ -51,9 +51,25 @@ def self.index_content_wrapper_key = WorkPackages::ActivitiesTab::StimulusContro def self.add_comment_wrapper_key = "work-packages-activities-tab-add-comment-component" delegate :index_content_wrapper_key, :add_comment_wrapper_key, to: :class + def filter_and_sorting_component + WorkPackages::ActivitiesTab::Journals::FilterAndSortingComponent.new(work_package:, filter:) + end + + def list_journals_component + WorkPackages::ActivitiesTab::Journals::IndexComponent.new(work_package:, journals:, filter:) + end + + def add_journal_component + WorkPackages::ActivitiesTab::Journals::NewComponent.new(work_package:, filter:, last_server_timestamp:) + end + + def error_stream_component + WorkPackages::ActivitiesTab::ErrorStreamComponent.new + end + private - attr_reader :work_package, :filter, :last_server_timestamp, :deferred + attr_reader :work_package, :journals, :filter, :last_server_timestamp def wrapper_data_attributes # rubocop:disable Metrics/AbcSize stimulus_controllers = { diff --git a/app/components/work_packages/activities_tab/journals/index_component.html.erb b/app/components/work_packages/activities_tab/journals/index_component.html.erb index 662bde4bd7aa..1894da309165 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/index_component.html.erb @@ -1,75 +1,41 @@ <%= - if deferred - helpers.turbo_frame_tag("work-package-activities-tab-content-older-journals") do - flex_layout do |older_journals_container| - older_journals.each do |record| - older_journals_container.with_row do - if record.is_a?(Changeset) - render(WorkPackages::ActivitiesTab::Journals::RevisionComponent.new(changeset: record, filter:)) - else - render( - WorkPackages::ActivitiesTab::Journals::ItemComponent.new( - journal: record, filter:, - grouped_emoji_reactions: wp_journals_grouped_emoji_reactions[record.id] - ) - ) + component_wrapper(class: "work-packages-activities-tab-journals-index-component") do + flex_layout(data: { test_selector: "op-wp-journals-#{filter}-#{journal_sorting}" }) do |journals_index_wrapper_container| + journals_index_wrapper_container.with_row( + classes: "work-packages-activities-tab-journals-index-component--journals-inner-container", + mb: inner_container_margin_bottom + ) do + flex_layout( + id: insert_target_modifier_id, + data: { test_selector: "op-wp-journals-container" } + ) do |journals_index_container| + if empty_state? + journals_index_container.with_row(mt: 2, mb: 3) do + render(WorkPackages::ActivitiesTab::Journals::EmptyComponent.new) end end - end - end - end - else - component_wrapper(class: "work-packages-activities-tab-journals-index-component") do - flex_layout(data: { test_selector: "op-wp-journals-#{filter}-#{journal_sorting}" }) do |journals_index_wrapper_container| - journals_index_wrapper_container.with_row( - classes: "work-packages-activities-tab-journals-index-component--journals-inner-container", - mb: inner_container_margin_bottom - ) do - flex_layout( - id: insert_target_modifier_id, - data: { test_selector: "op-wp-journals-container" } - ) do |journals_index_container| - if empty_state? - journals_index_container.with_row(mt: 2, mb: 3) do - render( - WorkPackages::ActivitiesTab::Journals::EmptyComponent.new - ) - end - end - if !journal_sorting_desc? && journals.count > MAX_RECENT_JOURNALS - journals_index_container.with_row do - helpers.turbo_frame_tag("work-package-activities-tab-content-older-journals", src: work_package_activities_path(work_package, filter:, deferred: true)) - end + if journal_sorting_asc? + journals_index_container.with_row do + render(infinite_scroll_component) end + end - recent_journals.each do |record| - journals_index_container.with_row do - if record.is_a?(Changeset) - render(WorkPackages::ActivitiesTab::Journals::RevisionComponent.new(changeset: record, filter:)) - else - render( - WorkPackages::ActivitiesTab::Journals::ItemComponent.new( - journal: record, filter:, - grouped_emoji_reactions: wp_journals_grouped_emoji_reactions[record.id] - ) - ) - end - end - end + journals_index_container.with_row do + render(page_component) + end - if journal_sorting_desc? && journals.count > MAX_RECENT_JOURNALS - journals_index_container.with_row do - helpers.turbo_frame_tag("work-package-activities-tab-content-older-journals", src: work_package_activities_path(work_package, filter:, deferred: true)) - end + if journal_sorting_desc? + journals_index_container.with_row do + render(infinite_scroll_component) end end end + end - unless empty_state? - journals_index_wrapper_container - .with_row(classes: "work-packages-activities-tab-journals-index-component--stem-connection") - end + unless empty_state? + journals_index_wrapper_container + .with_row(classes: "work-packages-activities-tab-journals-index-component--stem-connection") end end end diff --git a/app/components/work_packages/activities_tab/journals/index_component.rb b/app/components/work_packages/activities_tab/journals/index_component.rb index 04fc8edb5d86..af4d63d873ad 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.rb +++ b/app/components/work_packages/activities_tab/journals/index_component.rb @@ -32,89 +32,36 @@ module WorkPackages module ActivitiesTab module Journals class IndexComponent < ApplicationComponent - MAX_RECENT_JOURNALS = 30 - include ApplicationHelper include OpPrimer::ComponentHelpers include OpTurbo::Streamable include WorkPackages::ActivitiesTab::SharedHelpers - def initialize(work_package:, filter: :all, deferred: false) + def initialize(work_package:, journals:, filter: :all) super @work_package = work_package + @journals = journals @filter = filter - @deferred = deferred - end - - private - - attr_reader :work_package, :filter, :deferred - - def insert_target_modified? - true - end - - def insert_target_modifier_id - "work-package-journal-days" - end - - def journal_sorting_desc? - journal_sorting == "desc" - end - - def base_journals - combine_and_sort_records(fetch_journals, fetch_revisions) - end - - def fetch_journals - API::V3::Activities::ActivityEagerLoadingWrapper.wrap( - work_package - .journals - .internal_visible - .includes(:user, :customizable_journals, :attachable_journals, :storable_journals, :notifications) - .reorder(version: journal_sorting) - .with_sequence_version - ) end - def fetch_revisions - work_package.changesets.includes(:user, :repository) + def infinite_scroll_component + WorkPackages::ActivitiesTab::Journals::InfiniteScrollComponent.new(work_package:) end - def combine_and_sort_records(journals, revisions) - (journals + revisions).sort_by do |record| - timestamp = record_timestamp(record) - journal_sorting_desc? ? [-timestamp, -record.id] : [timestamp, record.id] - end + def page_component + WorkPackages::ActivitiesTab::Journals::PageComponent.new(journals:, emoji_reactions:, page: 1, filter:) end - def record_timestamp(record) - if record.is_a?(API::V3::Activities::ActivityEagerLoadingWrapper) - record.created_at&.to_i - elsif record.is_a?(Changeset) - record.committed_on.to_i - end - end + def self.insert_target_modifier_id = "work-package-journal-days" + delegate :insert_target_modifier_id, to: :class - def journals - base_journals - end + private - def recent_journals - if journal_sorting_desc? - base_journals.first(MAX_RECENT_JOURNALS) - else - base_journals.last(MAX_RECENT_JOURNALS) - end - end + attr_reader :work_package, :journals, :filter - def older_journals - if journal_sorting_desc? - base_journals.drop(MAX_RECENT_JOURNALS) - else - base_journals.take(base_journals.size - MAX_RECENT_JOURNALS) - end + def insert_target_modified? + true end def journal_with_notes @@ -123,8 +70,8 @@ def journal_with_notes .where.not(notes: "") end - def wp_journals_grouped_emoji_reactions - @wp_journals_grouped_emoji_reactions ||= + def emoji_reactions + @emoji_reactions ||= EmojiReactions::GroupedQueries.grouped_work_package_journals_emoji_reactions_by_reactable(work_package) end diff --git a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb new file mode 100644 index 000000000000..269edae0380a --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb @@ -0,0 +1,25 @@ +<%= + component_wrapper( + data: { + controller: "infinite-scroll", + infinite_scroll_insert_target_id_value: insert_target_id, + infinite_scroll_page_url_value: page_streams_url, + infinite_scroll_current_page_value: 1, # initial page + infinite_scroll_is_last_page_value: false # initial value + } + ) do + flex_layout( + data: { infinite_scroll_target: "skeleton" }, + class: "work-packages-activities-tab-journals-item-component", + bg: :default + ) do |skeleton| + skeleton.with_row(mb: 2) do + render(Primer::Alpha::SkeletonBox.new(width: "25%", height: "40px")) + end + + skeleton.with_row(mb: 3) do + render(Primer::Alpha::SkeletonBox.new(width: "100%", height: "150px")) + end + end + end +%> diff --git a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb new file mode 100644 index 000000000000..a83d8495deb5 --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module WorkPackages + module ActivitiesTab + module Journals + class InfiniteScrollComponent < ApplicationComponent + include OpPrimer::ComponentHelpers + include OpTurbo::Streamable + + def initialize(work_package:) + super + @work_package = work_package + end + + private + + attr_reader :work_package + + def insert_target_id + WorkPackages::ActivitiesTab::Journals::IndexComponent.insert_target_modifier_id + end + + def page_streams_url + page_streams_work_package_activities_path(work_package, format: :turbo_stream) + end + end + end + end +end diff --git a/app/components/work_packages/activities_tab/journals/page_component.html.erb b/app/components/work_packages/activities_tab/journals/page_component.html.erb new file mode 100644 index 000000000000..cc02170f5fc8 --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/page_component.html.erb @@ -0,0 +1,20 @@ +<%= + component_wrapper do + flex_layout do |flex_container| + journals.each do |record| + flex_container.with_row do + if record.is_a?(Changeset) + render(WorkPackages::ActivitiesTab::Journals::RevisionComponent.new(changeset: record, filter:)) + else + render( + WorkPackages::ActivitiesTab::Journals::ItemComponent.new( + journal: record, filter:, + grouped_emoji_reactions: emoji_reactions.fetch(record.id, {}) + ) + ) + end + end + end + end + end +%> diff --git a/app/components/work_packages/activities_tab/journals/page_component.rb b/app/components/work_packages/activities_tab/journals/page_component.rb new file mode 100644 index 000000000000..86a4a6122608 --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/page_component.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module WorkPackages + module ActivitiesTab + module Journals + class PageComponent < ApplicationComponent + include OpPrimer::ComponentHelpers + include OpTurbo::Streamable + include WorkPackages::ActivitiesTab::SharedHelpers + + def initialize(journals:, emoji_reactions:, page:, filter: :all) + super + + @journals = journals + @emoji_reactions = emoji_reactions + @filter = filter + @page = page + end + + def render? + journals.any? + end + + def wrapper_uniq_by + page + end + + private + + attr_reader :journals, :emoji_reactions, :page, :filter + end + end + end +end diff --git a/app/components/work_packages/activities_tab/shared_helpers.rb b/app/components/work_packages/activities_tab/shared_helpers.rb index b9a03043d47d..b603fe4f610c 100644 --- a/app/components/work_packages/activities_tab/shared_helpers.rb +++ b/app/components/work_packages/activities_tab/shared_helpers.rb @@ -61,6 +61,14 @@ def journal_updated_at_formatted_time(journal) end end + def journal_sorting_asc? + journal_sorting == "asc" + end + + def journal_sorting_desc? + journal_sorting == "desc" + end + def journal_sorting User.current.preference&.comments_sorting || OpenProject::Configuration.default_comment_sort_order end diff --git a/app/controllers/concerns/op_turbo/component_stream.rb b/app/controllers/concerns/op_turbo/component_stream.rb index be727ca7ecbc..2332542f07d2 100644 --- a/app/controllers/concerns/op_turbo/component_stream.rb +++ b/app/controllers/concerns/op_turbo/component_stream.rb @@ -71,6 +71,15 @@ def modify_via_turbo_stream(component:, action:, status:, method: nil) ) end + def insert_via_turbo_stream(action:, component:, target_component:) + case action + when :append + append_via_turbo_stream(component:, target_component:) + when :prepend + prepend_via_turbo_stream(component:, target_component:) + end + end + def append_via_turbo_stream(component:, target_component:) turbo_streams << target_component.insert_as_turbo_stream(component:, view_context:, action: :append) end @@ -104,6 +113,14 @@ def render_flash_message_via_turbo_stream(message:, component: OpPrimer::FlashCo turbo_streams << instance.render_as_turbo_stream(view_context:, action: :flash) end + def set_dataset_attributes_via_turbo_stream(target, **attributes) + attributes.each do |attribute, value| + turbo_streams << OpTurbo::StreamComponent + .new(action: :set_dataset_attribute, target:, attribute:, value:) + .render_in(view_context) + end + end + def scroll_into_view_via_turbo_stream(target, behavior: :auto, block: :start) turbo_streams << OpTurbo::StreamComponent .new(action: :scroll_into_view, target:, behavior:, block:) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 7a538028f363..55c6310589f9 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -29,6 +29,7 @@ # ++ class WorkPackages::ActivitiesTabController < ApplicationController + include Pagy::Backend include OpTurbo::ComponentStream include FlashMessagesOutputSafetyHelper @@ -37,19 +38,44 @@ class WorkPackages::ActivitiesTabController < ApplicationController before_action :find_journal, only: %i[edit cancel_edit update toggle_reaction] before_action :set_filter before_action :authorize + before_action :initialize_pagination, only: %i[index page_streams update_filter update_sorting] def index render( WorkPackages::ActivitiesTab::IndexComponent.new( work_package: @work_package, + journals: @paginated_journals, filter: @filter, - last_server_timestamp: get_current_server_timestamp, - deferred: ActiveRecord::Type::Boolean.new.cast(params[:deferred]) + last_server_timestamp: get_current_server_timestamp ), layout: false ) end + def page_streams + insert_via_turbo_stream( + target_component: WorkPackages::ActivitiesTab::Journals::IndexComponent.new( + work_package: @work_package, + journals: Journal.none # we do not need to pass any journals here since we just want the component key + ), + component: WorkPackages::ActivitiesTab::Journals::PageComponent.new( + journals: @paginated_journals, + emoji_reactions: wp_journals_emoji_reactions, + page: @paginator.page, + filter: @filter + ), + action: journal_sorting.asc? ? :append : :prepend + ) + + set_dataset_attributes_via_turbo_stream( + WorkPackages::ActivitiesTab::Journals::InfiniteScrollComponent.wrapper_key, + infinite_scroll_current_page_value: @paginator.page, + infinite_scroll_is_last_page_value: @paginator.next.blank? + ) + + respond_with_turbo_streams + end + def update_streams set_last_server_timestamp_to_headers @@ -173,6 +199,48 @@ def toggle_reaction # rubocop:disable Metrics/AbcSize private + def initialize_pagination + @paginator, @paginated_journals = pagy_array(base_journals, items: 30) + # For UI display: if user wants "oldest first" UI, reverse the array + @paginated_journals = @paginated_journals.reverse if journal_sorting.asc? + end + + def base_journals + combine_and_sort_records(fetch_journals, fetch_revisions) + end + + def fetch_journals + API::V3::Activities::ActivityEagerLoadingWrapper.wrap(fetch_ar_journals) + end + + def fetch_ar_journals + @work_package + .journals + .internal_visible + .includes(:user, :customizable_journals, :attachable_journals, :storable_journals, :notifications) + .reorder(version: :desc) # Always fetch newest first for pagination + .with_sequence_version + end + + def fetch_revisions + @work_package.changesets.includes(:user, :repository) + end + + def combine_and_sort_records(journals, revisions) + (journals + revisions).sort_by do |record| + timestamp = record_timestamp(record) + [-timestamp, -record.id] # Always sort DESC (newest first) + end + end + + def record_timestamp(record) + if record.is_a?(API::V3::Activities::ActivityEagerLoadingWrapper) + record.created_at&.to_i + elsif record.is_a?(Changeset) + record.committed_on.to_i + end + end + def respond_with_error(error_message) respond_to do |format| # turbo_frame requests (tab is initially rendered and an error occured) are handled below @@ -218,7 +286,8 @@ def set_filter end def journal_sorting - User.current.preference&.comments_sorting || OpenProject::Configuration.default_comment_sort_order + ActiveSupport::StringInquirer + .new(User.current.preference&.comments_sorting || OpenProject::Configuration.default_comment_sort_order) end def sanitized_journal_notes @@ -287,6 +356,7 @@ def replace_whole_tab replace_via_turbo_stream( component: WorkPackages::ActivitiesTab::IndexComponent.new( work_package: @work_package, + journals: @paginated_journals, filter: @filter, last_server_timestamp: get_current_server_timestamp ) @@ -297,6 +367,7 @@ def update_index_component update_via_turbo_stream( component: WorkPackages::ActivitiesTab::Journals::IndexComponent.new( work_package: @work_package, + journals: @paginated_journals, filter: @filter ) ) @@ -339,7 +410,7 @@ def generate_time_based_update_streams(last_update_timestamp, editing_journals) rerender_updated_journals(journals, last_update_timestamp, grouped_emoji_reactions, editing_journals) rerender_journals_with_updated_notification(journals, last_update_timestamp, grouped_emoji_reactions, editing_journals) - append_or_prepend_journals(journals, last_update_timestamp, grouped_emoji_reactions) + insert_latest_journals_via_turbo_stream(journals, last_update_timestamp, grouped_emoji_reactions) if journals.present? remove_potential_empty_state @@ -348,13 +419,11 @@ def generate_time_based_update_streams(last_update_timestamp, editing_journals) end def generate_work_package_journals_emoji_reactions_update_streams - wp_journal_emoji_reactions = - EmojiReactions::GroupedQueries.grouped_work_package_journals_emoji_reactions_by_reactable(@work_package) @work_package.journals.each do |journal| update_via_turbo_stream( component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Reactions.new( journal:, - grouped_emoji_reactions: wp_journal_emoji_reactions[journal.id] || {} + grouped_emoji_reactions: wp_journals_emoji_reactions[journal.id] || {} ) ) end @@ -404,32 +473,21 @@ def rerender_journals_with_updated_notification(journals, last_update_timestamp, end end - def append_or_prepend_journals(journals, last_update_timestamp, grouped_emoji_reactions) - journals.where("created_at > ?", last_update_timestamp).find_each do |journal| - append_or_prepend_latest_journal_via_turbo_stream(journal, grouped_emoji_reactions.fetch(journal.id, {})) - end - end - - def append_or_prepend_latest_journal_via_turbo_stream(journal, grouped_emoji_reactions) + def insert_latest_journals_via_turbo_stream(journals, last_update_timestamp, emoji_reactions) target_component = WorkPackages::ActivitiesTab::Journals::IndexComponent.new( work_package: @work_package, + journals: Journal.none, # we do not need to pass any journals here since we just want the component key filter: @filter ) - component = WorkPackages::ActivitiesTab::Journals::ItemComponent.new( - journal:, filter: @filter, grouped_emoji_reactions: - ) - - stream_config = { - target_component:, - component: - } - - # Append or prepend the new journal depending on the sorting - if journal_sorting == "asc" - append_via_turbo_stream(**stream_config) - else - prepend_via_turbo_stream(**stream_config) + journals.where("created_at > ?", last_update_timestamp).find_each do |journal| + insert_via_turbo_stream( + target_component:, + component: WorkPackages::ActivitiesTab::Journals::ItemComponent.new( + journal:, filter: @filter, grouped_emoji_reactions: emoji_reactions.fetch(journal.id, {}) + ), + action: journal_sorting.asc? ? :append : :prepend + ) end end @@ -467,6 +525,11 @@ def update_activity_counter ) end + def wp_journals_emoji_reactions + @wp_journals_emoji_reactions ||= EmojiReactions::GroupedQueries + .grouped_work_package_journals_emoji_reactions_by_reactable(@work_package) + end + def grouped_emoji_reactions_for_journal EmojiReactions::GroupedQueries .grouped_emoji_reactions_by_reactable(reactable: @journal)[@journal.id] diff --git a/config/initializers/pagy.rb b/config/initializers/pagy.rb index a21dc471b3e9..d13c0b2f494d 100644 --- a/config/initializers/pagy.rb +++ b/config/initializers/pagy.rb @@ -61,7 +61,7 @@ # Array extra: Paginate arrays efficiently, avoiding expensive array-wrapping and without overriding # See https://ddnexus.github.io/pagy/docs/extras/array -# require 'pagy/extras/array' +require "pagy/extras/array" # Calendar extra: Add pagination filtering by calendar time unit (year, quarter, month, week, day) # See https://ddnexus.github.io/pagy/docs/extras/calendar @@ -105,7 +105,7 @@ # Keyset extra: Paginate with the Pagy keyset pagination technique # See https://ddnexus.github.io/pagy/docs/extras/keyset -# require 'pagy/extras/keyset' +require "pagy/extras/keyset" # Meilisearch extra: Paginate `Meilisearch` result objects # See https://ddnexus.github.io/pagy/docs/extras/meilisearch diff --git a/config/initializers/permissions.rb b/config/initializers/permissions.rb index abd50c324bda..8b9fe2fc7ba3 100644 --- a/config/initializers/permissions.rb +++ b/config/initializers/permissions.rb @@ -261,7 +261,7 @@ work_packages: %i[show index show_conflict_flash_message share_upsell], work_packages_api: [:get], "work_packages/reports": %i[report report_details], - "work_packages/activities_tab": %i[index update_streams update_sorting update_filter], + "work_packages/activities_tab": %i[index page_streams update_streams update_sorting update_filter], "work_packages/menus": %i[show], "work_packages/hover_card": %i[show], work_package_relations_tab: %i[index], diff --git a/config/routes.rb b/config/routes.rb index 7a7665d1432e..7549787d8e2d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -735,6 +735,7 @@ end collection do + get :page_streams get :update_streams get :update_filter # filter not persisted put :update_sorting # sorting is persisted From 447a45b575529337c601837c230fd5f5a91f6812 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 1 Oct 2025 19:00:49 +0300 Subject: [PATCH 04/24] Add infinite scrolling stimulus controller --- .../activities_tab/stimulus_controllers.rb | 1 + .../infinite_scroll_component.html.erb | 10 +- .../journals/infinite_scroll_component.rb | 11 +++ .../activities_tab_controller.rb | 7 +- .../infinite-scroll.controller.ts | 97 +++++++++++++++++++ frontend/src/stimulus/setup.ts | 2 + 6 files changed, 116 insertions(+), 12 deletions(-) create mode 100644 frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts diff --git a/app/components/concerns/work_packages/activities_tab/stimulus_controllers.rb b/app/components/concerns/work_packages/activities_tab/stimulus_controllers.rb index 6b6c2c000b97..0f994cf6b7bc 100644 --- a/app/components/concerns/work_packages/activities_tab/stimulus_controllers.rb +++ b/app/components/concerns/work_packages/activities_tab/stimulus_controllers.rb @@ -37,6 +37,7 @@ def auto_scrolling_stimulus_controller(suffix = nil) = "#{stimulus_controller_na def editor_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--editor#{suffix}" def index_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--index#{suffix}" def internal_comment_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--internal-comment#{suffix}" + def infinite_scroll_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--infinite-scroll#{suffix}" def polling_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--polling#{suffix}" def stems_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--stems#{suffix}" def quote_comments_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--quote-comment#{suffix}" diff --git a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb index 269edae0380a..cb2210183fd0 100644 --- a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb @@ -1,13 +1,5 @@ <%= - component_wrapper( - data: { - controller: "infinite-scroll", - infinite_scroll_insert_target_id_value: insert_target_id, - infinite_scroll_page_url_value: page_streams_url, - infinite_scroll_current_page_value: 1, # initial page - infinite_scroll_is_last_page_value: false # initial value - } - ) do + component_wrapper(data: wrapper_data_attributes) do flex_layout( data: { infinite_scroll_target: "skeleton" }, class: "work-packages-activities-tab-journals-item-component", diff --git a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb index a83d8495deb5..4773cc8d6fee 100644 --- a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb +++ b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb @@ -34,6 +34,7 @@ module Journals class InfiniteScrollComponent < ApplicationComponent include OpPrimer::ComponentHelpers include OpTurbo::Streamable + include WorkPackages::ActivitiesTab::StimulusControllers def initialize(work_package:) super @@ -44,6 +45,16 @@ def initialize(work_package:) attr_reader :work_package + def wrapper_data_attributes + { + controller: infinite_scroll_stimulus_controller, + infinite_scroll_stimulus_controller("-insert-target-id-value") => insert_target_id, + infinite_scroll_stimulus_controller("-page-value") => 1, + infinite_scroll_stimulus_controller("-is-last-page-value") => false, + infinite_scroll_stimulus_controller("-url-value") => page_streams_url + } + end + def insert_target_id WorkPackages::ActivitiesTab::Journals::IndexComponent.insert_target_modifier_id end diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 55c6310589f9..c0b1a770c6a3 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -32,6 +32,7 @@ class WorkPackages::ActivitiesTabController < ApplicationController include Pagy::Backend include OpTurbo::ComponentStream include FlashMessagesOutputSafetyHelper + include WorkPackages::ActivitiesTab::StimulusControllers before_action :find_work_package before_action :find_project @@ -64,13 +65,13 @@ def page_streams page: @paginator.page, filter: @filter ), - action: journal_sorting.asc? ? :append : :prepend + action: journal_sorting.desc? ? :append : :prepend ) set_dataset_attributes_via_turbo_stream( WorkPackages::ActivitiesTab::Journals::InfiniteScrollComponent.wrapper_key, - infinite_scroll_current_page_value: @paginator.page, - infinite_scroll_is_last_page_value: @paginator.next.blank? + infinite_scroll_stimulus_controller("-page-value") => @paginator.page, + infinite_scroll_stimulus_controller("-is-last-page-value") => @paginator.next.blank? ) respond_with_turbo_streams diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts new file mode 100644 index 000000000000..2681b812fced --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts @@ -0,0 +1,97 @@ +/* + * -- copyright + * OpenProject is an open source project management software. + * Copyright (C) 2023 the OpenProject GmbH + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version 3. + * + * OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: + * Copyright (C) 2006-2013 Jean-Philippe Lang + * Copyright (C) 2010-2013 the ChiliProject Team + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * See COPYRIGHT and LICENSE files for more details. + * ++ + */ + +import BaseController from './base.controller'; +import { TurboRequestsService } from 'core-app/core/turbo/turbo-requests.service'; +import { useIntersection } from 'stimulus-use'; + +export default class extends BaseController { + static values = { + url: String, + insertTargetId: String, + page: { type: Number, default: 1 }, + isLastPage: Boolean, + }; + + static targets = ['skeleton']; + + declare urlValue:string; + declare insertTargetIdValue:string; + declare pageValue:number; + declare isLastPageValue:boolean; + + declare readonly skeletonTarget:HTMLElement; + declare readonly hasSkeletonTarget:boolean; + + private updateInProgress = false; + private turboRequests:TurboRequestsService; + + connect() { + if (this.isLastPageValue) return; + + super.connect(); + void this.initializeTurboRequestService(); + useIntersection(this, { threshold: 1.0 }); + } + + async appear() { + if (this.updateInProgress || this.isLastPageValue) return; + + this.updateInProgress = true; + + await this.fetchNextPageStream() + .catch((error) => { + console.error('Error fetching next page:', error); + }).finally(() => { + this.updateInProgress = false; + }); + } + + private fetchNextPageStream():Promise<{ html:string, headers:Headers }> { + const url = this.preparePageStreamsUrl(); + return this.turboRequests.requestStream(url); + } + + private preparePageStreamsUrl():string { + const baseUrl = window.location.origin; + const url = new URL(this.urlValue, baseUrl); + this.pageValue += 1; + + url.searchParams.set('page', this.pageValue.toString()); + url.searchParams.set('filter', this.indexOutlet.filterValue); + + return url.toString(); + } + + private async initializeTurboRequestService() { + const context = await window.OpenProject.getPluginContext(); + this.turboRequests = context.services.turboRequests; + } +} diff --git a/frontend/src/stimulus/setup.ts b/frontend/src/stimulus/setup.ts index 174b3260c37d..5bfb6995e2da 100644 --- a/frontend/src/stimulus/setup.ts +++ b/frontend/src/stimulus/setup.ts @@ -24,6 +24,7 @@ import AutoScrollingController from './controllers/dynamic/work-packages/activit import PollingController from './controllers/dynamic/work-packages/activities-tab/polling.controller'; import StemsController from './controllers/dynamic/work-packages/activities-tab/stems.controller'; import EditorController from './controllers/dynamic/work-packages/activities-tab/editor.controller'; +import InfiniteScrollController from './controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller'; import AutoSubmit from '@stimulus-components/auto-submit'; import AutoThemeSwitcher from './controllers/auto-theme-switcher.controller'; @@ -64,6 +65,7 @@ OpenProjectStimulusApplication.preregister('work-packages--activities-tab--auto- OpenProjectStimulusApplication.preregister('work-packages--activities-tab--polling', PollingController); OpenProjectStimulusApplication.preregister('work-packages--activities-tab--stems', StemsController); OpenProjectStimulusApplication.preregister('work-packages--activities-tab--editor', EditorController); +OpenProjectStimulusApplication.preregister('work-packages--activities-tab--infinite-scroll', InfiniteScrollController); OpenProjectStimulusApplication.preregister('beforeunload', BeforeunloadController); OpenProjectStimulusApplication.preregister('auto-theme-switcher', AutoThemeSwitcher); OpenProjectStimulusApplication.preregister('external-links', ExternalLinksController); From aabd15be15bdebcc889d75a9feee42eba8ed6594 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 1 Oct 2025 20:37:23 +0300 Subject: [PATCH 05/24] Abort infinite scrolling for 1 page collections --- .../activities_tab/index_component.rb | 8 +++--- .../journals/index_component.html.erb | 25 +++++++++---------- .../journals/index_component.rb | 13 +++++----- .../infinite_scroll_component.html.erb | 2 +- .../journals/infinite_scroll_component.rb | 15 +++++------ .../activities_tab_controller.rb | 2 ++ .../infinite-scroll.controller.ts | 7 ++++++ 7 files changed, 39 insertions(+), 33 deletions(-) diff --git a/app/components/work_packages/activities_tab/index_component.rb b/app/components/work_packages/activities_tab/index_component.rb index 574870697f5c..8020ad4d4cf9 100644 --- a/app/components/work_packages/activities_tab/index_component.rb +++ b/app/components/work_packages/activities_tab/index_component.rb @@ -37,11 +37,12 @@ class IndexComponent < ApplicationComponent include WorkPackages::ActivitiesTab::SharedHelpers include WorkPackages::ActivitiesTab::StimulusControllers - def initialize(work_package:, journals:, last_server_timestamp:, filter: :all) + def initialize(work_package:, journals:, paginator:, last_server_timestamp:, filter: :all) super @work_package = work_package @journals = journals + @paginator = paginator @filter = filter @last_server_timestamp = last_server_timestamp end @@ -56,7 +57,8 @@ def filter_and_sorting_component end def list_journals_component - WorkPackages::ActivitiesTab::Journals::IndexComponent.new(work_package:, journals:, filter:) + WorkPackages::ActivitiesTab::Journals::IndexComponent + .new(work_package:, journals:, filter:, page: paginator.page, next_page: paginator.next) end def add_journal_component @@ -69,7 +71,7 @@ def error_stream_component private - attr_reader :work_package, :journals, :filter, :last_server_timestamp + attr_reader :work_package, :journals, :paginator, :filter, :last_server_timestamp def wrapper_data_attributes # rubocop:disable Metrics/AbcSize stimulus_controllers = { diff --git a/app/components/work_packages/activities_tab/journals/index_component.html.erb b/app/components/work_packages/activities_tab/journals/index_component.html.erb index 1894da309165..dc05d390db37 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/index_component.html.erb @@ -6,28 +6,27 @@ mb: inner_container_margin_bottom ) do flex_layout( - id: insert_target_modifier_id, data: { test_selector: "op-wp-journals-container" } ) do |journals_index_container| if empty_state? journals_index_container.with_row(mt: 2, mb: 3) do render(WorkPackages::ActivitiesTab::Journals::EmptyComponent.new) end - end - - if journal_sorting_asc? - journals_index_container.with_row do - render(infinite_scroll_component) + else + if journal_sorting_asc? + journals_index_container.with_row do + render(infinite_scroll_component) + end end - end - journals_index_container.with_row do - render(page_component) - end + journals_index_container.with_row(id: insert_target_modifier_id) do + render(page_component) + end - if journal_sorting_desc? - journals_index_container.with_row do - render(infinite_scroll_component) + if journal_sorting_desc? + journals_index_container.with_row do + render(infinite_scroll_component) + end end end end diff --git a/app/components/work_packages/activities_tab/journals/index_component.rb b/app/components/work_packages/activities_tab/journals/index_component.rb index af4d63d873ad..86d049ca7edd 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.rb +++ b/app/components/work_packages/activities_tab/journals/index_component.rb @@ -37,28 +37,27 @@ class IndexComponent < ApplicationComponent include OpTurbo::Streamable include WorkPackages::ActivitiesTab::SharedHelpers - def initialize(work_package:, journals:, filter: :all) + def initialize(work_package:, journals:, page: 1, next_page: nil, filter: :all) super @work_package = work_package @journals = journals + @page = page + @next_page = next_page @filter = filter end def infinite_scroll_component - WorkPackages::ActivitiesTab::Journals::InfiniteScrollComponent.new(work_package:) + WorkPackages::ActivitiesTab::Journals::InfiniteScrollComponent.new(work_package:, page:, next_page:) end def page_component - WorkPackages::ActivitiesTab::Journals::PageComponent.new(journals:, emoji_reactions:, page: 1, filter:) + WorkPackages::ActivitiesTab::Journals::PageComponent.new(journals:, emoji_reactions:, page:, filter:) end - def self.insert_target_modifier_id = "work-package-journal-days" - delegate :insert_target_modifier_id, to: :class - private - attr_reader :work_package, :journals, :filter + attr_reader :work_package, :journals, :page, :next_page, :filter def insert_target_modified? true diff --git a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb index cb2210183fd0..b28b82e7b214 100644 --- a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb @@ -1,7 +1,7 @@ <%= component_wrapper(data: wrapper_data_attributes) do flex_layout( - data: { infinite_scroll_target: "skeleton" }, + data: { "#{infinite_scroll_stimulus_controller}-target": "skeleton" }, class: "work-packages-activities-tab-journals-item-component", bg: :default ) do |skeleton| diff --git a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb index 4773cc8d6fee..dd0a89327813 100644 --- a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb +++ b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb @@ -36,29 +36,26 @@ class InfiniteScrollComponent < ApplicationComponent include OpTurbo::Streamable include WorkPackages::ActivitiesTab::StimulusControllers - def initialize(work_package:) + def initialize(work_package:, page: 1, next_page: nil) super @work_package = work_package + @page = page + @next_page = next_page end private - attr_reader :work_package + attr_reader :work_package, :page, :next_page def wrapper_data_attributes { controller: infinite_scroll_stimulus_controller, - infinite_scroll_stimulus_controller("-insert-target-id-value") => insert_target_id, - infinite_scroll_stimulus_controller("-page-value") => 1, - infinite_scroll_stimulus_controller("-is-last-page-value") => false, + infinite_scroll_stimulus_controller("-page-value") => page, + infinite_scroll_stimulus_controller("-is-last-page-value") => next_page.blank?, infinite_scroll_stimulus_controller("-url-value") => page_streams_url } end - def insert_target_id - WorkPackages::ActivitiesTab::Journals::IndexComponent.insert_target_modifier_id - end - def page_streams_url page_streams_work_package_activities_path(work_package, format: :turbo_stream) end diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index c0b1a770c6a3..5a2ac8ce24b0 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -46,6 +46,7 @@ def index WorkPackages::ActivitiesTab::IndexComponent.new( work_package: @work_package, journals: @paginated_journals, + paginator: @paginator, filter: @filter, last_server_timestamp: get_current_server_timestamp ), @@ -358,6 +359,7 @@ def replace_whole_tab component: WorkPackages::ActivitiesTab::IndexComponent.new( work_package: @work_package, journals: @paginated_journals, + paginator: @paginator, filter: @filter, last_server_timestamp: get_current_server_timestamp ) diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts index 2681b812fced..652c388b1393 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts @@ -74,6 +74,13 @@ export default class extends BaseController { }); } + isLastPageValueChanged(isLastPage:boolean, _previousValue:boolean) { + if (isLastPage) { + (this.element as HTMLElement).hidden = true; + if (this.hasSkeletonTarget) this.skeletonTarget.remove(); + } + } + private fetchNextPageStream():Promise<{ html:string, headers:Headers }> { const url = this.preparePageStreamsUrl(); return this.turboRequests.requestStream(url); From aa7901d3cba6a724157daebdd02090a823f3b486 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 1 Oct 2025 22:18:34 +0300 Subject: [PATCH 06/24] Preserve scroll during infinite scroll page render --- .../journals/index_component.rb | 3 + .../journals/infinite_scroll_component.rb | 5 + .../infinite-scroll.controller.ts | 39 +++++++- .../activities-tab/services/dom_helpers.ts | 98 +++++++++++++++++++ frontend/src/typings/turbo.d.ts | 7 ++ 5 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts diff --git a/app/components/work_packages/activities_tab/journals/index_component.rb b/app/components/work_packages/activities_tab/journals/index_component.rb index 86d049ca7edd..f0f6fad66b8a 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.rb +++ b/app/components/work_packages/activities_tab/journals/index_component.rb @@ -55,6 +55,9 @@ def page_component WorkPackages::ActivitiesTab::Journals::PageComponent.new(journals:, emoji_reactions:, page:, filter:) end + def self.insert_target_modifier_id = "#{wrapper_key}-pages" + delegate :insert_target_modifier_id, to: :class + private attr_reader :work_package, :journals, :page, :next_page, :filter diff --git a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb index dd0a89327813..b09fda125578 100644 --- a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb +++ b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb @@ -50,12 +50,17 @@ def initialize(work_package:, page: 1, next_page: nil) def wrapper_data_attributes { controller: infinite_scroll_stimulus_controller, + infinite_scroll_stimulus_controller("-insert-target-id-value") => insert_target_id, infinite_scroll_stimulus_controller("-page-value") => page, infinite_scroll_stimulus_controller("-is-last-page-value") => next_page.blank?, infinite_scroll_stimulus_controller("-url-value") => page_streams_url } end + def insert_target_id + WorkPackages::ActivitiesTab::Journals::IndexComponent.insert_target_modifier_id + end + def page_streams_url page_streams_work_package_activities_path(work_package, format: :turbo_stream) end diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts index 652c388b1393..cc38cf63210c 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts @@ -28,9 +28,11 @@ * ++ */ -import BaseController from './base.controller'; import { TurboRequestsService } from 'core-app/core/turbo/turbo-requests.service'; +import type { TurboBeforeStreamRenderEvent } from 'core-typings/turbo'; import { useIntersection } from 'stimulus-use'; +import BaseController from './base.controller'; +import { DomHelpers } from './services/dom_helpers'; export default class extends BaseController { static values = { @@ -52,6 +54,8 @@ export default class extends BaseController { private updateInProgress = false; private turboRequests:TurboRequestsService; + private abortController = new AbortController(); + private pageStreamHandler?:(_event:TurboBeforeStreamRenderEvent) => void; connect() { if (this.isLastPageValue) return; @@ -59,6 +63,14 @@ export default class extends BaseController { super.connect(); void this.initializeTurboRequestService(); useIntersection(this, { threshold: 1.0 }); + + this.setupScrollPreservation(); + } + + disconnect() { + super.disconnect(); + this.abortController.abort(); + if (this.pageStreamHandler) this.pageStreamHandler = undefined; } async appear() { @@ -81,6 +93,31 @@ export default class extends BaseController { } } + setupScrollPreservation() { + if (!this.scrollableContainer || this.pageStreamHandler) return; + + const { signal } = this.abortController; + + this.pageStreamHandler = (event:TurboBeforeStreamRenderEvent) => { + event.preventDefault(); + + const stream = event.detail.newStream; + const scrollContainer = this.scrollableContainer!; + + if (stream.target.includes(this.insertTargetIdValue)) { + const isPrepend = stream.action === 'prepend'; + void DomHelpers.keepScroll(scrollContainer, isPrepend, () => { + event.detail.render(stream); + return Promise.resolve(); + }); + } else { + event.detail.render(stream); + } + }; + + document.addEventListener('turbo:before-stream-render', this.pageStreamHandler as EventListener, { signal }); + } + private fetchNextPageStream():Promise<{ html:string, headers:Headers }> { const url = this.preparePageStreamsUrl(); return this.turboRequests.requestStream(url); diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts new file mode 100644 index 000000000000..9427231a7588 --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts @@ -0,0 +1,98 @@ +/* +* -- copyright +* OpenProject is an open source project management software. +* Copyright (C) 2023 the OpenProject GmbH +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License version 3. +* +* OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +* Copyright (C) 2006-2013 Jean-Philippe Lang +* Copyright (C) 2010-2013 the ChiliProject Team +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License +* as published by the Free Software Foundation; either version 2 +* of the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +* +* See COPYRIGHT and LICENSE files for more details. +* ++ +*/ + +export namespace DomHelpers { + /** + * Preserves scroll position during DOM manipulations that change content height. + * + * This is crucial for infinite scroll UX - when older content is prepended above + * the current view, we need to adjust scroll position so the user doesn't suddenly + * jump to a different part of the content. + * + * @param {HTMLElement} container - The scrollable container + * @param {boolean} top - Whether content is being added at the top (prepend) + * true: adjust scroll for prepended content + * false: maintain position for appended content + * @param {Function} fn - Async function that performs the DOM manipulation + * + * Algorithm: + * - Capture current scroll position and total height BEFORE DOM changes + * - Execute the DOM manipulation (adding/removing content) + * - Calculate height difference and adjust scroll position accordingly + * + * For prepend: scrollTop + heightDifference (push view down by added content height) + * For append: maintain original scrollTop (new content below doesn't affect view) + */ + export async function keepScroll(container:HTMLElement, top:boolean, fn:() => Promise) { + pauseInertiaScroll(container); + + const scrollTop = container.scrollTop; + const scrollHeight = container.scrollHeight; + + await fn(); + + if (top) { + container.scrollTop = scrollTop + (container.scrollHeight - scrollHeight); + } else { + container.scrollTop = scrollTop; + } + } + + /** + * Temporarily pauses inertial/momentum scrolling to prevent scroll position drift. + * + * CRITICAL for iOS Safari and other mobile browsers with momentum scrolling! + * + * The Problem: + * When programmatically setting scrollTop during an active momentum scroll, + * the browser continues the inertial scroll AFTER our position adjustment, + * causing the scroll position to drift away from our intended target. + * + * The Solution: + * - Set overflow: hidden to immediately stop any momentum scrolling + * - Use requestAnimationFrame to restore overflow on next frame + * - This gives us a clean slate to set the exact scroll position + * + * Without this, scroll preservation would be unreliable on mobile devices, + * especially during infinite scroll when users are actively scrolling up. + * + * Credit: This technique was adapted from Basecamp's Campfire implementation + * where they solved similar scroll position issues in their real-time chat. + * + * @param {Element} container - The scrollable container to pause momentum on + */ + function pauseInertiaScroll(container:HTMLElement) { + container.style.overflow = 'hidden'; + + requestAnimationFrame(() => { + container.style.overflow = ''; + }); + } +} diff --git a/frontend/src/typings/turbo.d.ts b/frontend/src/typings/turbo.d.ts index e7a6389d8e42..ef9bcfed1462 100644 --- a/frontend/src/typings/turbo.d.ts +++ b/frontend/src/typings/turbo.d.ts @@ -6,3 +6,10 @@ export interface TurboStreamElement extends HTMLElement { action:string; target:string; } + +export interface TurboBeforeStreamRenderEvent extends CustomEvent { + detail:{ + newStream:TurboStreamElement; + render:(stream:TurboStreamElement) => void; + }; +} From 4f47f31f997d9c1f24c93868f79e4e26fd92bcb9 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 1 Oct 2025 22:49:51 +0300 Subject: [PATCH 07/24] Lower the intersection threshold to 25% --- .../infinite-scroll.controller.ts | 12 +++++--- .../activities-tab/services/dom_helpers.ts | 28 +++++++++++++++++-- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts index cc38cf63210c..168d3f6ca80c 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts @@ -62,15 +62,14 @@ export default class extends BaseController { super.connect(); void this.initializeTurboRequestService(); - useIntersection(this, { threshold: 1.0 }); + useIntersection(this, { threshold: 0.25 }); this.setupScrollPreservation(); } disconnect() { super.disconnect(); - this.abortController.abort(); - if (this.pageStreamHandler) this.pageStreamHandler = undefined; + this.tearDownScrollPreservation(); } async appear() { @@ -93,7 +92,7 @@ export default class extends BaseController { } } - setupScrollPreservation() { + private setupScrollPreservation() { if (!this.scrollableContainer || this.pageStreamHandler) return; const { signal } = this.abortController; @@ -118,6 +117,11 @@ export default class extends BaseController { document.addEventListener('turbo:before-stream-render', this.pageStreamHandler as EventListener, { signal }); } + private tearDownScrollPreservation() { + this.abortController.abort(); + if (this.pageStreamHandler) this.pageStreamHandler = undefined; + } + private fetchNextPageStream():Promise<{ html:string, headers:Headers }> { const url = this.preparePageStreamsUrl(); return this.turboRequests.requestStream(url); diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts index 9427231a7588..b5cf04a90a16 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts @@ -26,6 +26,31 @@ * * See COPYRIGHT and LICENSE files for more details. * ++ +* +* Copyright (c) 37signals, LLC +* +* Permission is hereby granted, free of charge, to any person obtaining +* a copy of this software and associated documentation files (the +* "Software"), to deal in the Software without restriction, including +* without limitation the rights to use, copy, modify, merge, publish, +* distribute, sublicense, and/or sell copies of the Software, and to +* permit persons to whom the Software is furnished to do so, subject to +* the following conditions: +* +* The above copyright notice and this permission notice shall be +* included in all copies or substantial portions of the Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE +* LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +* OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION +* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +* +* See also the original source: +* * https://github.com/basecamp/once-campfire/blob/5c0526eaf7f83d129472a520fa673bfcb9c2f397/MIT-LICENSE +* * https://github.com/basecamp/once-campfire/blob/5c0526eaf7f83d129472a520fa673bfcb9c2f397/app/javascript/helpers/dom_helpers.js#L44-L65 */ export namespace DomHelpers { @@ -83,9 +108,6 @@ export namespace DomHelpers { * Without this, scroll preservation would be unreliable on mobile devices, * especially during infinite scroll when users are actively scrolling up. * - * Credit: This technique was adapted from Basecamp's Campfire implementation - * where they solved similar scroll position issues in their real-time chat. - * * @param {Element} container - The scrollable container to pause momentum on */ function pauseInertiaScroll(container:HTMLElement) { From c1d070ab6f7c7db0f8b21c8c121e63cfc456b6ad Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 2 Oct 2025 13:03:11 +0300 Subject: [PATCH 08/24] Extract `journal_sorting.{asc?,desc?}` inquiry --- .../journal_sorting_inquirable.rb | 42 +++++++++++++++++++ .../journals/index_component.html.erb | 4 +- .../journals/index_component.rb | 6 +-- .../activities_tab/shared_helpers.rb | 18 +++----- .../activities_tab_controller.rb | 6 +-- 5 files changed, 52 insertions(+), 24 deletions(-) create mode 100644 app/components/concerns/work_packages/activities_tab/journal_sorting_inquirable.rb diff --git a/app/components/concerns/work_packages/activities_tab/journal_sorting_inquirable.rb b/app/components/concerns/work_packages/activities_tab/journal_sorting_inquirable.rb new file mode 100644 index 000000000000..9d5f90ae3bab --- /dev/null +++ b/app/components/concerns/work_packages/activities_tab/journal_sorting_inquirable.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +# ++ + +module WorkPackages + module ActivitiesTab + module JournalSortingInquirable + extend ActiveSupport::Concern + + def journal_sorting + ActiveSupport::StringInquirer + .new(User.current.preference&.comments_sorting || OpenProject::Configuration.default_comment_sort_order) + end + end + end +end diff --git a/app/components/work_packages/activities_tab/journals/index_component.html.erb b/app/components/work_packages/activities_tab/journals/index_component.html.erb index dc05d390db37..7f2c2f973978 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/index_component.html.erb @@ -13,7 +13,7 @@ render(WorkPackages::ActivitiesTab::Journals::EmptyComponent.new) end else - if journal_sorting_asc? + if journal_sorting.asc? journals_index_container.with_row do render(infinite_scroll_component) end @@ -23,7 +23,7 @@ render(page_component) end - if journal_sorting_desc? + if journal_sorting.desc? journals_index_container.with_row do render(infinite_scroll_component) end diff --git a/app/components/work_packages/activities_tab/journals/index_component.rb b/app/components/work_packages/activities_tab/journals/index_component.rb index f0f6fad66b8a..43c047646192 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.rb +++ b/app/components/work_packages/activities_tab/journals/index_component.rb @@ -82,11 +82,7 @@ def empty_state? end def inner_container_margin_bottom - if journal_sorting_desc? - 3 - else - 0 - end + journal_sorting.desc? ? 3 : 0 end end end diff --git a/app/components/work_packages/activities_tab/shared_helpers.rb b/app/components/work_packages/activities_tab/shared_helpers.rb index b603fe4f610c..0a9350045601 100644 --- a/app/components/work_packages/activities_tab/shared_helpers.rb +++ b/app/components/work_packages/activities_tab/shared_helpers.rb @@ -31,6 +31,12 @@ module WorkPackages module ActivitiesTab module SharedHelpers + extend ActiveSupport::Concern + + included do + include WorkPackages::ActivitiesTab::JournalSortingInquirable + end + def truncated_user_name(user, hover_card: false) helpers.primer_link_to_user(user, scheme: :primary, font_weight: :bold, hover_card:) end @@ -61,18 +67,6 @@ def journal_updated_at_formatted_time(journal) end end - def journal_sorting_asc? - journal_sorting == "asc" - end - - def journal_sorting_desc? - journal_sorting == "desc" - end - - def journal_sorting - User.current.preference&.comments_sorting || OpenProject::Configuration.default_comment_sort_order - end - def activity_url(journal) "#{project_work_package_url(journal.journable.project, journal.journable)}/activity#{activity_anchor(journal)}" end diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 5a2ac8ce24b0..d9d6b908b1cb 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -32,6 +32,7 @@ class WorkPackages::ActivitiesTabController < ApplicationController include Pagy::Backend include OpTurbo::ComponentStream include FlashMessagesOutputSafetyHelper + include WorkPackages::ActivitiesTab::JournalSortingInquirable include WorkPackages::ActivitiesTab::StimulusControllers before_action :find_work_package @@ -287,11 +288,6 @@ def set_filter @filter = (params[:filter] || params.dig(:journal, :filter))&.to_sym || :all end - def journal_sorting - ActiveSupport::StringInquirer - .new(User.current.preference&.comments_sorting || OpenProject::Configuration.default_comment_sort_order) - end - def sanitized_journal_notes WorkPackages::ActivitiesTab::InternalCommentMentionsSanitizer.sanitize(@work_package, journal_params[:notes]) end From cab44aa53a4412a0c54180aaba1925b70e3c3d7c Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 2 Oct 2025 13:05:30 +0300 Subject: [PATCH 09/24] Specify scrollable container as the intended target whose bounding rectangle will be considered the viewport --- .../activities-tab/infinite-scroll.controller.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts index 168d3f6ca80c..9ebaa44ad631 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts @@ -62,7 +62,6 @@ export default class extends BaseController { super.connect(); void this.initializeTurboRequestService(); - useIntersection(this, { threshold: 0.25 }); this.setupScrollPreservation(); } @@ -96,12 +95,14 @@ export default class extends BaseController { if (!this.scrollableContainer || this.pageStreamHandler) return; const { signal } = this.abortController; + const scrollContainer = this.scrollableContainer; + + useIntersection(this, { root: scrollContainer }); this.pageStreamHandler = (event:TurboBeforeStreamRenderEvent) => { event.preventDefault(); const stream = event.detail.newStream; - const scrollContainer = this.scrollableContainer!; if (stream.target.includes(this.insertTargetIdValue)) { const isPrepend = stream.action === 'prepend'; From 05c20e4401919177df308f19827fd376f70e2457 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 3 Oct 2025 11:56:33 +0300 Subject: [PATCH 10/24] Tidy up function calls including guard clauses --- .../activities-tab/infinite-scroll.controller.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts index 9ebaa44ad631..fcd4f28641d4 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts @@ -62,7 +62,6 @@ export default class extends BaseController { super.connect(); void this.initializeTurboRequestService(); - this.setupScrollPreservation(); } @@ -103,8 +102,9 @@ export default class extends BaseController { event.preventDefault(); const stream = event.detail.newStream; + const insertTargetId = this.insertTargetIdValue; - if (stream.target.includes(this.insertTargetIdValue)) { + if (insertTargetId && stream.target.includes(insertTargetId)) { const isPrepend = stream.action === 'prepend'; void DomHelpers.keepScroll(scrollContainer, isPrepend, () => { event.detail.render(stream); From 2484b4852526d1b2bce6d32be31a0b4387c831b1 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 3 Oct 2025 20:40:42 +0300 Subject: [PATCH 11/24] Switch from infinite scrolling to managed lazy loaded pages Akin to lazy frames but with better intersection threshold control --- .../activities_tab/stimulus_controllers.rb | 2 +- .../activities_tab/index_component.rb | 2 +- .../journals/index_component.html.erb | 14 +----- .../journals/index_component.rb | 27 ++++++++--- .../infinite_scroll_component.html.erb | 17 ------- .../journals/lazy_page_component.html.erb | 47 +++++++++++++++++++ ...ll_component.rb => lazy_page_component.rb} | 29 +++++++----- .../concerns/op_turbo/component_stream.rb | 8 ---- .../activities_tab_controller.rb | 17 ++----- ....controller.ts => lazy-page.controller.ts} | 46 ++++++++---------- frontend/src/stimulus/setup.ts | 4 +- 11 files changed, 111 insertions(+), 102 deletions(-) delete mode 100644 app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb create mode 100644 app/components/work_packages/activities_tab/journals/lazy_page_component.html.erb rename app/components/work_packages/activities_tab/journals/{infinite_scroll_component.rb => lazy_page_component.rb} (71%) rename frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/{infinite-scroll.controller.ts => lazy-page.controller.ts} (80%) diff --git a/app/components/concerns/work_packages/activities_tab/stimulus_controllers.rb b/app/components/concerns/work_packages/activities_tab/stimulus_controllers.rb index 0f994cf6b7bc..71c087766435 100644 --- a/app/components/concerns/work_packages/activities_tab/stimulus_controllers.rb +++ b/app/components/concerns/work_packages/activities_tab/stimulus_controllers.rb @@ -37,7 +37,7 @@ def auto_scrolling_stimulus_controller(suffix = nil) = "#{stimulus_controller_na def editor_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--editor#{suffix}" def index_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--index#{suffix}" def internal_comment_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--internal-comment#{suffix}" - def infinite_scroll_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--infinite-scroll#{suffix}" + def lazy_page_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--lazy-page#{suffix}" def polling_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--polling#{suffix}" def stems_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--stems#{suffix}" def quote_comments_stimulus_controller(suffix = nil) = "#{stimulus_controller_namespace}--quote-comment#{suffix}" diff --git a/app/components/work_packages/activities_tab/index_component.rb b/app/components/work_packages/activities_tab/index_component.rb index 8020ad4d4cf9..e926d749b743 100644 --- a/app/components/work_packages/activities_tab/index_component.rb +++ b/app/components/work_packages/activities_tab/index_component.rb @@ -58,7 +58,7 @@ def filter_and_sorting_component def list_journals_component WorkPackages::ActivitiesTab::Journals::IndexComponent - .new(work_package:, journals:, filter:, page: paginator.page, next_page: paginator.next) + .new(work_package:, journals:, filter:, paginator:) end def add_journal_component diff --git a/app/components/work_packages/activities_tab/journals/index_component.html.erb b/app/components/work_packages/activities_tab/journals/index_component.html.erb index 7f2c2f973978..763773a9fd8f 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/index_component.html.erb @@ -13,20 +13,8 @@ render(WorkPackages::ActivitiesTab::Journals::EmptyComponent.new) end else - if journal_sorting.asc? - journals_index_container.with_row do - render(infinite_scroll_component) - end - end - journals_index_container.with_row(id: insert_target_modifier_id) do - render(page_component) - end - - if journal_sorting.desc? - journals_index_container.with_row do - render(infinite_scroll_component) - end + pages.map { concat render(it) } end end end diff --git a/app/components/work_packages/activities_tab/journals/index_component.rb b/app/components/work_packages/activities_tab/journals/index_component.rb index 43c047646192..2bce3326e501 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.rb +++ b/app/components/work_packages/activities_tab/journals/index_component.rb @@ -37,30 +37,43 @@ class IndexComponent < ApplicationComponent include OpTurbo::Streamable include WorkPackages::ActivitiesTab::SharedHelpers - def initialize(work_package:, journals:, page: 1, next_page: nil, filter: :all) + def initialize(work_package:, journals:, paginator:, filter: :all) super @work_package = work_package @journals = journals - @page = page - @next_page = next_page + @paginator = paginator @filter = filter end - def infinite_scroll_component - WorkPackages::ActivitiesTab::Journals::InfiniteScrollComponent.new(work_package:, page:, next_page:) + def pages + current_page = paginator.page + + result = (1..paginator.pages).map do |page| + if page == current_page + page_component(page) + else + lazy_page_component(page) + end + end + + result.tap { it.reverse! if journal_sorting.asc? && paginator.pages > 1 } end - def page_component + def page_component(page) WorkPackages::ActivitiesTab::Journals::PageComponent.new(journals:, emoji_reactions:, page:, filter:) end + def lazy_page_component(page) + WorkPackages::ActivitiesTab::Journals::LazyPageComponent.new(work_package:, page:) + end + def self.insert_target_modifier_id = "#{wrapper_key}-pages" delegate :insert_target_modifier_id, to: :class private - attr_reader :work_package, :journals, :page, :next_page, :filter + attr_reader :work_package, :journals, :paginator, :filter def insert_target_modified? true diff --git a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb b/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb deleted file mode 100644 index b28b82e7b214..000000000000 --- a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.html.erb +++ /dev/null @@ -1,17 +0,0 @@ -<%= - component_wrapper(data: wrapper_data_attributes) do - flex_layout( - data: { "#{infinite_scroll_stimulus_controller}-target": "skeleton" }, - class: "work-packages-activities-tab-journals-item-component", - bg: :default - ) do |skeleton| - skeleton.with_row(mb: 2) do - render(Primer::Alpha::SkeletonBox.new(width: "25%", height: "40px")) - end - - skeleton.with_row(mb: 3) do - render(Primer::Alpha::SkeletonBox.new(width: "100%", height: "150px")) - end - end - end -%> diff --git a/app/components/work_packages/activities_tab/journals/lazy_page_component.html.erb b/app/components/work_packages/activities_tab/journals/lazy_page_component.html.erb new file mode 100644 index 000000000000..f02e84f185af --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/lazy_page_component.html.erb @@ -0,0 +1,47 @@ +<%# -- copyright +OpenProject is an open source project management software. +Copyright (C) the OpenProject GmbH + +This program is free software; you can redistribute it and/or +modify it under the terms of the GNU General Public License version 3. + +OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +Copyright (C) 2006-2013 Jean-Philippe Lang +Copyright (C) 2010-2013 the ChiliProject Team + +This program is free software; you can redistribute it and/or +modify it under the terms of the GNU General Public License +as published by the Free Software Foundation; either version 2 +of the License, or (at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program; if not, write to the Free Software +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +See COPYRIGHT and LICENSE files for more details. + +++#%> + +<%= + component_wrapper(data: wrapper_data_attributes) do + flex_layout( + class: "work-packages-activities-tab-journals-item-component", + bg: :default + ) do |skeleton| + 5.times do + skeleton.with_row(mb: 2) do + render(Primer::Alpha::SkeletonBox.new(width: "25%", height: "40px")) + end + + skeleton.with_row(mb: 3) do + render(Primer::Alpha::SkeletonBox.new(width: "100%", height: "150px")) + end + end + end + end +%> diff --git a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb b/app/components/work_packages/activities_tab/journals/lazy_page_component.rb similarity index 71% rename from app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb rename to app/components/work_packages/activities_tab/journals/lazy_page_component.rb index b09fda125578..de80f189dc1c 100644 --- a/app/components/work_packages/activities_tab/journals/infinite_scroll_component.rb +++ b/app/components/work_packages/activities_tab/journals/lazy_page_component.rb @@ -31,36 +31,39 @@ module WorkPackages module ActivitiesTab module Journals - class InfiniteScrollComponent < ApplicationComponent + class LazyPageComponent < ApplicationComponent include OpPrimer::ComponentHelpers include OpTurbo::Streamable include WorkPackages::ActivitiesTab::StimulusControllers - def initialize(work_package:, page: 1, next_page: nil) + def initialize(work_package:, page:) super @work_package = work_package @page = page - @next_page = next_page + end + + def self.wrapper_key + WorkPackages::ActivitiesTab::Journals::PageComponent.wrapper_key + end + + def wrapper_uniq_by + page end private - attr_reader :work_package, :page, :next_page + attr_reader :work_package, :page def wrapper_data_attributes { - controller: infinite_scroll_stimulus_controller, - infinite_scroll_stimulus_controller("-insert-target-id-value") => insert_target_id, - infinite_scroll_stimulus_controller("-page-value") => page, - infinite_scroll_stimulus_controller("-is-last-page-value") => next_page.blank?, - infinite_scroll_stimulus_controller("-url-value") => page_streams_url + controller: lazy_page_stimulus_controller, + lazy_page_stimulus_controller("-insert-target-id-value") => wrapper_key, + lazy_page_stimulus_controller("-page-value") => page, + lazy_page_stimulus_controller("-url-value") => page_streams_url, + lazy_page_stimulus_controller("-is-loaded-value") => false } end - def insert_target_id - WorkPackages::ActivitiesTab::Journals::IndexComponent.insert_target_modifier_id - end - def page_streams_url page_streams_work_package_activities_path(work_package, format: :turbo_stream) end diff --git a/app/controllers/concerns/op_turbo/component_stream.rb b/app/controllers/concerns/op_turbo/component_stream.rb index 2332542f07d2..54d26aa95a0e 100644 --- a/app/controllers/concerns/op_turbo/component_stream.rb +++ b/app/controllers/concerns/op_turbo/component_stream.rb @@ -113,14 +113,6 @@ def render_flash_message_via_turbo_stream(message:, component: OpPrimer::FlashCo turbo_streams << instance.render_as_turbo_stream(view_context:, action: :flash) end - def set_dataset_attributes_via_turbo_stream(target, **attributes) - attributes.each do |attribute, value| - turbo_streams << OpTurbo::StreamComponent - .new(action: :set_dataset_attribute, target:, attribute:, value:) - .render_in(view_context) - end - end - def scroll_into_view_via_turbo_stream(target, behavior: :auto, block: :start) turbo_streams << OpTurbo::StreamComponent .new(action: :scroll_into_view, target:, behavior:, block:) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index d9d6b908b1cb..b037318a9426 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -56,24 +56,13 @@ def index end def page_streams - insert_via_turbo_stream( - target_component: WorkPackages::ActivitiesTab::Journals::IndexComponent.new( - work_package: @work_package, - journals: Journal.none # we do not need to pass any journals here since we just want the component key - ), + replace_via_turbo_stream( component: WorkPackages::ActivitiesTab::Journals::PageComponent.new( journals: @paginated_journals, emoji_reactions: wp_journals_emoji_reactions, page: @paginator.page, filter: @filter - ), - action: journal_sorting.desc? ? :append : :prepend - ) - - set_dataset_attributes_via_turbo_stream( - WorkPackages::ActivitiesTab::Journals::InfiniteScrollComponent.wrapper_key, - infinite_scroll_stimulus_controller("-page-value") => @paginator.page, - infinite_scroll_stimulus_controller("-is-last-page-value") => @paginator.next.blank? + ) ) respond_with_turbo_streams @@ -367,6 +356,7 @@ def update_index_component component: WorkPackages::ActivitiesTab::Journals::IndexComponent.new( work_package: @work_package, journals: @paginated_journals, + paginator: @paginator, filter: @filter ) ) @@ -476,6 +466,7 @@ def insert_latest_journals_via_turbo_stream(journals, last_update_timestamp, emo target_component = WorkPackages::ActivitiesTab::Journals::IndexComponent.new( work_package: @work_package, journals: Journal.none, # we do not need to pass any journals here since we just want the component key + paginator: nil, filter: @filter ) diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts similarity index 80% rename from frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts rename to frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts index fcd4f28641d4..e190426c1a82 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts @@ -1,7 +1,7 @@ /* * -- copyright * OpenProject is an open source project management software. - * Copyright (C) 2023 the OpenProject GmbH + * Copyright (C) the OpenProject GmbH * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License version 3. @@ -34,31 +34,26 @@ import { useIntersection } from 'stimulus-use'; import BaseController from './base.controller'; import { DomHelpers } from './services/dom_helpers'; -export default class extends BaseController { +export default class LazyPageController extends BaseController { static values = { url: String, insertTargetId: String, page: { type: Number, default: 1 }, - isLastPage: Boolean, + isLoaded: { type: Boolean, default: false }, }; - static targets = ['skeleton']; - declare urlValue:string; declare insertTargetIdValue:string; declare pageValue:number; - declare isLastPageValue:boolean; - - declare readonly skeletonTarget:HTMLElement; - declare readonly hasSkeletonTarget:boolean; + declare isLoadedValue:boolean; - private updateInProgress = false; private turboRequests:TurboRequestsService; private abortController = new AbortController(); private pageStreamHandler?:(_event:TurboBeforeStreamRenderEvent) => void; + private stopObserving?:() => void; connect() { - if (this.isLastPageValue) return; + if (this.isLoadedValue) return; super.connect(); void this.initializeTurboRequestService(); @@ -68,35 +63,28 @@ export default class extends BaseController { disconnect() { super.disconnect(); this.tearDownScrollPreservation(); + this.stopObserving?.(); } - async appear() { - if (this.updateInProgress || this.isLastPageValue) return; + async appear(entry:IntersectionObserverEntry, observer:IntersectionObserver) { + observer.unobserve(entry.target); // observe and emit `appear()` callback just once + if (this.isLoadedValue) return; - this.updateInProgress = true; - - await this.fetchNextPageStream() + await this.fetchPageStream() .catch((error) => { console.error('Error fetching next page:', error); }).finally(() => { - this.updateInProgress = false; + this.isLoadedValue = true; }); } - isLastPageValueChanged(isLastPage:boolean, _previousValue:boolean) { - if (isLastPage) { - (this.element as HTMLElement).hidden = true; - if (this.hasSkeletonTarget) this.skeletonTarget.remove(); - } - } - private setupScrollPreservation() { if (!this.scrollableContainer || this.pageStreamHandler) return; const { signal } = this.abortController; const scrollContainer = this.scrollableContainer; - useIntersection(this, { root: scrollContainer }); + this.setupIntersectionObserver({ root: scrollContainer }); this.pageStreamHandler = (event:TurboBeforeStreamRenderEvent) => { event.preventDefault(); @@ -118,12 +106,17 @@ export default class extends BaseController { document.addEventListener('turbo:before-stream-render', this.pageStreamHandler as EventListener, { signal }); } + private setupIntersectionObserver({ root }:{ root:HTMLElement }) { + const [_observe, unobserve] = useIntersection(this, { root, threshold: 0.25, dispatchEvent: false }); + this.stopObserving = unobserve; + } + private tearDownScrollPreservation() { this.abortController.abort(); if (this.pageStreamHandler) this.pageStreamHandler = undefined; } - private fetchNextPageStream():Promise<{ html:string, headers:Headers }> { + private fetchPageStream():Promise<{ html:string, headers:Headers }> { const url = this.preparePageStreamsUrl(); return this.turboRequests.requestStream(url); } @@ -131,7 +124,6 @@ export default class extends BaseController { private preparePageStreamsUrl():string { const baseUrl = window.location.origin; const url = new URL(this.urlValue, baseUrl); - this.pageValue += 1; url.searchParams.set('page', this.pageValue.toString()); url.searchParams.set('filter', this.indexOutlet.filterValue); diff --git a/frontend/src/stimulus/setup.ts b/frontend/src/stimulus/setup.ts index 5bfb6995e2da..f2d125a389f4 100644 --- a/frontend/src/stimulus/setup.ts +++ b/frontend/src/stimulus/setup.ts @@ -24,7 +24,7 @@ import AutoScrollingController from './controllers/dynamic/work-packages/activit import PollingController from './controllers/dynamic/work-packages/activities-tab/polling.controller'; import StemsController from './controllers/dynamic/work-packages/activities-tab/stems.controller'; import EditorController from './controllers/dynamic/work-packages/activities-tab/editor.controller'; -import InfiniteScrollController from './controllers/dynamic/work-packages/activities-tab/infinite-scroll.controller'; +import LazyPageController from './controllers/dynamic/work-packages/activities-tab/lazy-page.controller'; import AutoSubmit from '@stimulus-components/auto-submit'; import AutoThemeSwitcher from './controllers/auto-theme-switcher.controller'; @@ -65,7 +65,7 @@ OpenProjectStimulusApplication.preregister('work-packages--activities-tab--auto- OpenProjectStimulusApplication.preregister('work-packages--activities-tab--polling', PollingController); OpenProjectStimulusApplication.preregister('work-packages--activities-tab--stems', StemsController); OpenProjectStimulusApplication.preregister('work-packages--activities-tab--editor', EditorController); -OpenProjectStimulusApplication.preregister('work-packages--activities-tab--infinite-scroll', InfiniteScrollController); +OpenProjectStimulusApplication.preregister('work-packages--activities-tab--lazy-page', LazyPageController); OpenProjectStimulusApplication.preregister('beforeunload', BeforeunloadController); OpenProjectStimulusApplication.preregister('auto-theme-switcher', AutoThemeSwitcher); OpenProjectStimulusApplication.preregister('external-links', ExternalLinksController); From 40404ab084c1e96c92c14cd845043db459d07880 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 6 Oct 2025 14:10:05 +0300 Subject: [PATCH 12/24] Delay loading to allow rapid scrolling without triggering loads --- .../activities-tab/lazy-page.controller.ts | 40 ++++++++++++++----- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts index e190426c1a82..d37714c1f68d 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts @@ -40,17 +40,20 @@ export default class LazyPageController extends BaseController { insertTargetId: String, page: { type: Number, default: 1 }, isLoaded: { type: Boolean, default: false }, + loadDelayMs: { type: Number, default: 75 }, }; declare urlValue:string; declare insertTargetIdValue:string; declare pageValue:number; declare isLoadedValue:boolean; + declare loadDelayMsValue:number; private turboRequests:TurboRequestsService; private abortController = new AbortController(); private pageStreamHandler?:(_event:TurboBeforeStreamRenderEvent) => void; private stopObserving?:() => void; + private loadTimeout?:number; connect() { if (this.isLoadedValue) return; @@ -63,19 +66,26 @@ export default class LazyPageController extends BaseController { disconnect() { super.disconnect(); this.tearDownScrollPreservation(); + this.cancelPendingLoad(); this.stopObserving?.(); } - async appear(entry:IntersectionObserverEntry, observer:IntersectionObserver) { - observer.unobserve(entry.target); // observe and emit `appear()` callback just once + appear() { if (this.isLoadedValue) return; - await this.fetchPageStream() - .catch((error) => { - console.error('Error fetching next page:', error); - }).finally(() => { - this.isLoadedValue = true; - }); + // Delay loading to allow rapid scrolling without triggering loads + this.loadTimeout = window.setTimeout(() => { + void this.fetchPageStream() + .catch((error) => { + console.error('Error fetching next page:', error); + }).finally(() => { + this.isLoadedValue = true; + }); + }, this.loadDelayMsValue); + } + + disappear() { + this.cancelPendingLoad(); // Cancel load if element leaves viewport before delay expires } private setupScrollPreservation() { @@ -107,7 +117,12 @@ export default class LazyPageController extends BaseController { } private setupIntersectionObserver({ root }:{ root:HTMLElement }) { - const [_observe, unobserve] = useIntersection(this, { root, threshold: 0.25, dispatchEvent: false }); + const [_observe, unobserve] = useIntersection(this, { + root, + threshold: 0.25, + dispatchEvent: false + }); + this.stopObserving = unobserve; } @@ -135,4 +150,11 @@ export default class LazyPageController extends BaseController { const context = await window.OpenProject.getPluginContext(); this.turboRequests = context.services.turboRequests; } + + private cancelPendingLoad() { + if (this.loadTimeout) { + window.clearTimeout(this.loadTimeout); + this.loadTimeout = undefined; + } + } } From 79673c5a8962e42db5922f28713992ff1044f12c Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Mon, 6 Oct 2025 18:08:50 +0300 Subject: [PATCH 13/24] Support paginated deep linking of comments --- .../activities_tab_controller.rb | 40 ++++++++++++++++++- .../activity-base.controller.ts | 18 ++++++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index b037318a9426..1449722c1e68 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -192,11 +192,49 @@ def toggle_reaction # rubocop:disable Metrics/AbcSize private def initialize_pagination - @paginator, @paginated_journals = pagy_array(base_journals, items: 30) + anchor_type, target_journal_id = extract_target_journal_id + + @paginator, @paginated_journals = + if anchor_type && target_journal_id + pagy_array_for_target_journal(anchor_type, target_journal_id) + else + pagy_array(base_journals) + end + # For UI display: if user wants "oldest first" UI, reverse the array @paginated_journals = @paginated_journals.reverse if journal_sorting.asc? end + def extract_target_journal_id + anchor = params[:anchor] # e.g., "comment-78758" (without #) + return nil unless anchor + + match = anchor.match(/^(comment|activity)-(\d+)$/) + match && match.length == 3 ? [match[1].inquiry, match[2].to_i] : [] + end + + def pagy_array_for_target_journal(anchor_type, target_journal_id) + journals = base_journals + + target_index = journals.find_index do |record| + if anchor_type.comment? + record.id == target_journal_id + elsif anchor_type.activity? + record.sequence_version == target_journal_id + else + false + end + end + + if target_index + target_page = (target_index / Pagy::DEFAULT[:limit]) + 1 + pagy_array(journals, page: target_page) + else + # Journal might be filtered out or deleted - fallback to page 1 + pagy_array(journals, page: 1) + end + end + def base_journals combine_and_sort_records(fetch_journals, fetch_revisions) end diff --git a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts index 76711600c944..518a0d337625 100644 --- a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts +++ b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts @@ -59,6 +59,22 @@ export class ActivityPanelBaseController extends UntilDestroyedMixin implements } ngOnInit():void { - this.turboFrameSrc = `${this.pathHelper.staticBase}/work_packages/${this.workPackageId}/activities`; + this.turboFrameSrc = this.buildTurboFrameSrc(); + } + + protected buildTurboFrameSrc(): string { + const baseUrl = `${this.pathHelper.staticBase}/work_packages/${this.workPackageId}/activities`; + const hash = window.location.hash; + + if (hash) { + // Extract anchor (e.g., "#comment-123" or "#activity-456") + const anchorMatch = hash.match(/^#(comment|activity)-(\d+)$/i); + if (anchorMatch && anchorMatch.length === 3) { + const anchor = hash.slice(1); // Remove # prefix + return `${baseUrl}?anchor=${encodeURIComponent(anchor)}`; + } + } + + return baseUrl; } } From 45b41784fb7ea23ec815eeaf81a9f6048885bc55 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Oct 2025 10:22:47 +0300 Subject: [PATCH 14/24] DRY up activity anchor url parsing --- .../activity-base.controller.ts | 15 ++-- .../auto-scrolling.controller.ts | 25 +++---- .../activities-tab/services/url-helpers.ts | 69 +++++++++++++++++++ 3 files changed, 85 insertions(+), 24 deletions(-) create mode 100644 frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/url-helpers.ts diff --git a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts index 518a0d337625..daf27fc25617 100644 --- a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts +++ b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts @@ -36,6 +36,7 @@ import { WpSingleViewService } from 'core-app/features/work-packages/routing/wp- import { BrowserDetector } from 'core-app/core/browser/browser-detector.service'; import { DeviceService } from 'core-app/core/browser/device.service'; import { PathHelperService } from 'core-app/core/path-helper/path-helper.service'; +import { UrlHelpers } from 'core-stimulus/controllers/dynamic/work-packages/activities-tab/services/url-helpers'; @Directive() export class ActivityPanelBaseController extends UntilDestroyedMixin implements OnInit { @@ -62,17 +63,13 @@ export class ActivityPanelBaseController extends UntilDestroyedMixin implements this.turboFrameSrc = this.buildTurboFrameSrc(); } - protected buildTurboFrameSrc(): string { + protected buildTurboFrameSrc():string { const baseUrl = `${this.pathHelper.staticBase}/work_packages/${this.workPackageId}/activities`; - const hash = window.location.hash; + const anchorInfo = UrlHelpers.extractActivityAnchor(window.location.hash); - if (hash) { - // Extract anchor (e.g., "#comment-123" or "#activity-456") - const anchorMatch = hash.match(/^#(comment|activity)-(\d+)$/i); - if (anchorMatch && anchorMatch.length === 3) { - const anchor = hash.slice(1); // Remove # prefix - return `${baseUrl}?anchor=${encodeURIComponent(anchor)}`; - } + if (anchorInfo) { + const anchor = `${anchorInfo.type}-${anchorInfo.id}`; + return `${baseUrl}?anchor=${encodeURIComponent(anchor)}`; } return baseUrl; diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/auto-scrolling.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/auto-scrolling.controller.ts index f78d76fec1c5..e1d6e4f8d127 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/auto-scrolling.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/auto-scrolling.controller.ts @@ -29,16 +29,12 @@ */ import BaseController from './base.controller'; - -enum AnchorType { - Comment = 'comment', - Activity = 'activity', -} +import { UrlHelpers, ActivityAnchorType, ActivityAnchor } from './services/url-helpers'; interface CustomEventWithIdParam extends Event { params:{ id:string; - anchorName:AnchorType; + anchorName:ActivityAnchorType; }; } @@ -59,7 +55,7 @@ export default class AutoScrollingController extends BaseController { // not using the scrollToActivity method here as it is causing flickering issues // in case of a setAnchor click, we can go for a direct scroll approach const scrollableContainer = this.scrollableContainer; - const activityElement = this.getActivityAnchorElement(anchorName, activityId); + const activityElement = this.getActivityAnchorElement({ type: anchorName, id: activityId }); const locationHash = `#${anchorName}-${activityId}`; if (scrollableContainer && activityElement) { @@ -120,12 +116,12 @@ export default class AutoScrollingController extends BaseController { } private handleInitialScroll() { - const anchorTypeRegex = new RegExp(`#(${AnchorType.Comment}|${AnchorType.Activity})-(\\d+)`, 'i'); - const activityIdMatch = window.location.hash.match(anchorTypeRegex); // Ex. [ "#comment-80", "comment", "80" ] + const hash = window.location.hash; + const anchorInfo = UrlHelpers.extractActivityAnchor(hash); - if (activityIdMatch && activityIdMatch.length === 3) { - const activityElement = this.getActivityAnchorElement(activityIdMatch[1] as AnchorType, activityIdMatch[2]); - this.brieflyHighlightAndResetUrl(activityElement, window.location.hash); + if (anchorInfo) { + const activityElement = this.getActivityAnchorElement(anchorInfo); + this.brieflyHighlightAndResetUrl(activityElement, hash); this.scrollToActivity(activityElement); } else if (this.indexOutlet.sortingAscending && (!this.isMobile() || this.isWithinNotificationCenter())) { this.scrollToBottom(); @@ -179,7 +175,6 @@ export default class AutoScrollingController extends BaseController { let timeoutId:ReturnType; const observer = new MutationObserver(() => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-argument clearTimeout(timeoutId); timeoutId = setTimeout(() => { @@ -211,8 +206,8 @@ export default class AutoScrollingController extends BaseController { } } - private getActivityAnchorElement(activityAnchorName:AnchorType, activityId:string):HTMLElement | null { - return document.querySelector(`[data-anchor-${activityAnchorName}-id="${activityId}"]`); + private getActivityAnchorElement(activityAnchor:ActivityAnchor):HTMLElement | null { + return document.querySelector(`[data-anchor-${activityAnchor.type}-id="${activityAnchor.id}"]`); } private get inputContainer():HTMLElement | null { diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/url-helpers.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/url-helpers.ts new file mode 100644 index 000000000000..ab9761be18ce --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/url-helpers.ts @@ -0,0 +1,69 @@ +/* + * -- copyright + * OpenProject is an open source project management software. + * Copyright (C) 2023 the OpenProject GmbH + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version 3. + * + * OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: + * Copyright (C) 2006-2013 Jean-Philippe Lang + * Copyright (C) 2010-2013 the ChiliProject Team + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * See COPYRIGHT and LICENSE files for more details. + * ++ + */ + +/* eslint-disable no-unused-vars */ +export enum ActivityAnchorType { + Comment = 'comment', + Activity = 'activity', +} +/* eslint-enable no-unused-vars */ + +export interface ActivityAnchor { + type:ActivityAnchorType; + id:string; +} + +const anchorTypeRegex = new RegExp(`#(${ActivityAnchorType.Comment}|${ActivityAnchorType.Activity})-(\\d+)`, 'i'); + +export namespace UrlHelpers { + /** + * Extracts activity anchor information from a URL anchor string. + * + * @param anchor - The anchor string to parse (e.g., "#comment-80", "#activity-45") + * @returns An ActivityAnchor object containing the type and id, or null if parsing fails + * + * @example + * ```typescript + * const result = extractActivityAnchor("#comment-80"); + * // Returns: { type: "comment", id: "80" } + * ``` + */ + export function extractActivityAnchor(anchor:string):ActivityAnchor | null { + const activityIdMatch = anchor.match(anchorTypeRegex); // Ex. [ "#comment-80", "comment", "80" ] + + if (activityIdMatch?.[1] && activityIdMatch?.[2]) { + const type = activityIdMatch[1]; + if (Object.values(ActivityAnchorType).includes(type as ActivityAnchorType)) { + return { type: type as ActivityAnchorType, id: activityIdMatch[2] }; + } + } + return null; + } +} From 815bebad789bf36dabd065ca8e6205683b3b4959 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Oct 2025 11:24:13 +0300 Subject: [PATCH 15/24] Prefer structured `URL` builder --- .../activity-panel/activity-base.controller.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts index daf27fc25617..01cb30fbf20f 100644 --- a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts +++ b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/activity-panel/activity-base.controller.ts @@ -64,14 +64,14 @@ export class ActivityPanelBaseController extends UntilDestroyedMixin implements } protected buildTurboFrameSrc():string { - const baseUrl = `${this.pathHelper.staticBase}/work_packages/${this.workPackageId}/activities`; + const baseUrl = window.location.origin; + const url = new URL(`${this.pathHelper.staticBase}/work_packages/${this.workPackageId}/activities`, baseUrl); const anchorInfo = UrlHelpers.extractActivityAnchor(window.location.hash); if (anchorInfo) { - const anchor = `${anchorInfo.type}-${anchorInfo.id}`; - return `${baseUrl}?anchor=${encodeURIComponent(anchor)}`; + url.searchParams.set('anchor', `${anchorInfo.type}-${anchorInfo.id}`); } - return baseUrl; + return url.toString(); } } From 406d83d32e46c88d09518563325f6503f99cef8b Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Oct 2025 14:33:02 +0300 Subject: [PATCH 16/24] Decouple observer setup from scroll setup --- .../activities-tab/lazy-page.controller.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts index d37714c1f68d..93dd0e50bfda 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts @@ -60,6 +60,7 @@ export default class LazyPageController extends BaseController { super.connect(); void this.initializeTurboRequestService(); + this.startObserving(); this.setupScrollPreservation(); } @@ -89,20 +90,18 @@ export default class LazyPageController extends BaseController { } private setupScrollPreservation() { - if (!this.scrollableContainer || this.pageStreamHandler) return; + if (this.pageStreamHandler) return; const { signal } = this.abortController; const scrollContainer = this.scrollableContainer; - this.setupIntersectionObserver({ root: scrollContainer }); - this.pageStreamHandler = (event:TurboBeforeStreamRenderEvent) => { event.preventDefault(); const stream = event.detail.newStream; const insertTargetId = this.insertTargetIdValue; - if (insertTargetId && stream.target.includes(insertTargetId)) { + if (scrollContainer && insertTargetId && stream.target.includes(insertTargetId)) { const isPrepend = stream.action === 'prepend'; void DomHelpers.keepScroll(scrollContainer, isPrepend, () => { event.detail.render(stream); @@ -116,7 +115,9 @@ export default class LazyPageController extends BaseController { document.addEventListener('turbo:before-stream-render', this.pageStreamHandler as EventListener, { signal }); } - private setupIntersectionObserver({ root }:{ root:HTMLElement }) { + private startObserving(root = this.scrollableContainer) { + if (!root) return; + const [_observe, unobserve] = useIntersection(this, { root, threshold: 0.25, From 13853fdedd9edefbd96f766a9103ddda133257cc Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Oct 2025 14:36:12 +0300 Subject: [PATCH 17/24] Re-initialize pagination JIT for tab replacement Pick up any changes to sorting or filtering preferences --- app/controllers/work_packages/activities_tab_controller.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 1449722c1e68..963ec51c6167 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -40,7 +40,7 @@ class WorkPackages::ActivitiesTabController < ApplicationController before_action :find_journal, only: %i[edit cancel_edit update toggle_reaction] before_action :set_filter before_action :authorize - before_action :initialize_pagination, only: %i[index page_streams update_filter update_sorting] + before_action :initialize_pagination, only: %i[index page_streams] def index render( @@ -378,6 +378,7 @@ def handle_internal_server_error(error) end def replace_whole_tab + initialize_pagination # re-initialize pagination to pick up changes to sorting/filtering replace_via_turbo_stream( component: WorkPackages::ActivitiesTab::IndexComponent.new( work_package: @work_package, @@ -390,6 +391,7 @@ def replace_whole_tab end def update_index_component + initialize_pagination # re-initialize pagination to pick up changes to sorting/filtering update_via_turbo_stream( component: WorkPackages::ActivitiesTab::Journals::IndexComponent.new( work_package: @work_package, From 2cffdfc3b235d23276c2d8bac84bb46e7a426d88 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Oct 2025 15:43:41 +0300 Subject: [PATCH 18/24] Fix timing issue in Activities#type_comment for lazy-loaded pages Add `wait_for_network_idle` at the start of `type_comment` to ensure all pending Turbo Stream rendering completes before attempting to interact with the form. This is needed as the lazy pages now intercept turbo stream rendering. --- spec/support/components/work_packages/activities.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/support/components/work_packages/activities.rb b/spec/support/components/work_packages/activities.rb index ab6e48675669..c088f9ec6510 100644 --- a/spec/support/components/work_packages/activities.rb +++ b/spec/support/components/work_packages/activities.rb @@ -234,6 +234,9 @@ def expect_unsaved_content(text) end def type_comment(text) + # Wait for any pending Turbo Stream updates to complete + wait_for_network_idle + begin open_new_comment_editor if page.find_test_selector("op-open-work-package-journal-form-trigger") rescue Capybara::ElementNotFound From 448b32849c699bf07e6e0c520d5f5c7b09287709 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Oct 2025 18:37:47 +0300 Subject: [PATCH 19/24] Create enough comments > 1 page --- spec/features/activities/work_package/activities_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/activities/work_package/activities_spec.rb b/spec/features/activities/work_package/activities_spec.rb index 378e165e87b1..f4e471d8b1ea 100644 --- a/spec/features/activities/work_package/activities_spec.rb +++ b/spec/features/activities/work_package/activities_spec.rb @@ -1035,7 +1035,7 @@ let(:work_package) { create(:work_package, project:, author: admin) } # create enough comments to make the journal container scrollable - 20.times do |i| + 25.times do |i| let!(:"comment_#{i + 1}") do create(:work_package_journal, user: admin, notes: "Comment #{i + 1}", journable: work_package, version: i + 2) end From 790f9a44d6ef85f3a28aa2e669f0abc499921494 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 7 Oct 2025 20:31:14 +0300 Subject: [PATCH 20/24] Extract work package activity tab pagination --- .../activities_tab_controller.rb | 79 +----- .../work_packages/activities_tab/paginator.rb | 129 +++++++++ spec/factories/work_package_factory.rb | 1 - .../activities_tab/paginator_spec.rb | 253 ++++++++++++++++++ 4 files changed, 383 insertions(+), 79 deletions(-) create mode 100644 app/services/work_packages/activities_tab/paginator.rb create mode 100644 spec/services/work_packages/activities_tab/paginator_spec.rb diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index 963ec51c6167..d92dbf5b3843 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -29,7 +29,6 @@ # ++ class WorkPackages::ActivitiesTabController < ApplicationController - include Pagy::Backend include OpTurbo::ComponentStream include FlashMessagesOutputSafetyHelper include WorkPackages::ActivitiesTab::JournalSortingInquirable @@ -192,83 +191,7 @@ def toggle_reaction # rubocop:disable Metrics/AbcSize private def initialize_pagination - anchor_type, target_journal_id = extract_target_journal_id - - @paginator, @paginated_journals = - if anchor_type && target_journal_id - pagy_array_for_target_journal(anchor_type, target_journal_id) - else - pagy_array(base_journals) - end - - # For UI display: if user wants "oldest first" UI, reverse the array - @paginated_journals = @paginated_journals.reverse if journal_sorting.asc? - end - - def extract_target_journal_id - anchor = params[:anchor] # e.g., "comment-78758" (without #) - return nil unless anchor - - match = anchor.match(/^(comment|activity)-(\d+)$/) - match && match.length == 3 ? [match[1].inquiry, match[2].to_i] : [] - end - - def pagy_array_for_target_journal(anchor_type, target_journal_id) - journals = base_journals - - target_index = journals.find_index do |record| - if anchor_type.comment? - record.id == target_journal_id - elsif anchor_type.activity? - record.sequence_version == target_journal_id - else - false - end - end - - if target_index - target_page = (target_index / Pagy::DEFAULT[:limit]) + 1 - pagy_array(journals, page: target_page) - else - # Journal might be filtered out or deleted - fallback to page 1 - pagy_array(journals, page: 1) - end - end - - def base_journals - combine_and_sort_records(fetch_journals, fetch_revisions) - end - - def fetch_journals - API::V3::Activities::ActivityEagerLoadingWrapper.wrap(fetch_ar_journals) - end - - def fetch_ar_journals - @work_package - .journals - .internal_visible - .includes(:user, :customizable_journals, :attachable_journals, :storable_journals, :notifications) - .reorder(version: :desc) # Always fetch newest first for pagination - .with_sequence_version - end - - def fetch_revisions - @work_package.changesets.includes(:user, :repository) - end - - def combine_and_sort_records(journals, revisions) - (journals + revisions).sort_by do |record| - timestamp = record_timestamp(record) - [-timestamp, -record.id] # Always sort DESC (newest first) - end - end - - def record_timestamp(record) - if record.is_a?(API::V3::Activities::ActivityEagerLoadingWrapper) - record.created_at&.to_i - elsif record.is_a?(Changeset) - record.committed_on.to_i - end + @paginator, @paginated_journals = WorkPackages::ActivitiesTab::Paginator.paginate(@work_package, params) end def respond_with_error(error_message) diff --git a/app/services/work_packages/activities_tab/paginator.rb b/app/services/work_packages/activities_tab/paginator.rb new file mode 100644 index 000000000000..eccb65d2dec8 --- /dev/null +++ b/app/services/work_packages/activities_tab/paginator.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +class WorkPackages::ActivitiesTab::Paginator + include Pagy::Backend + include WorkPackages::ActivitiesTab::JournalSortingInquirable + + def self.paginate(work_package, params = {}) + new(work_package, params).call + end + + def initialize(work_package, params = {}) + @work_package = work_package + @params = params + end + + def call + anchor_type, target_record_id = extract_target_record_id + + pagy, records = + if anchor_type && target_record_id + pagy_array_for_target_journal(anchor_type, target_record_id) + else + pagy_array(base_journals) + end + + # For UI display: if user wants "oldest first" UI, reverse the array + records = records.reverse if journal_sorting.asc? + + [pagy, records] + end + + private + + attr_reader :work_package, :params + + def extract_target_record_id + anchor = params[:anchor] # e.g., "comment-78758" (without #) + return nil unless anchor + + match = anchor.match(/^(comment|activity)-(\d+)$/) + match && match.length == 3 ? [match[1].inquiry, match[2].to_i] : [] + end + + def pagy_array_for_target_journal(anchor_type, target_record_id) + journals = base_journals + + target_index = journals.find_index do |record| + if anchor_type.comment? + record.id == target_record_id + elsif anchor_type.activity? + record.sequence_version == target_record_id + else + false + end + end + + if target_index + target_page = (target_index / Pagy::DEFAULT[:limit]) + 1 + pagy_array(journals, page: target_page) + else + # Journal might be filtered out or deleted - fallback to page 1 + pagy_array(journals, page: 1) + end + end + + def base_journals + combine_and_sort_records(fetch_journals, fetch_revisions) + end + + def fetch_journals + API::V3::Activities::ActivityEagerLoadingWrapper.wrap(fetch_ar_journals) + end + + def fetch_ar_journals + work_package + .journals + .internal_visible + .includes(:user, :customizable_journals, :attachable_journals, :storable_journals, :notifications) + .reorder(version: :desc) # Always fetch newest first for pagination + .with_sequence_version + end + + def fetch_revisions + work_package.changesets.includes(:user, :repository) + end + + def combine_and_sort_records(journals, revisions) + (journals + revisions).sort_by do |record| + timestamp = record_timestamp(record) + [-timestamp, -record.id] # Always sort DESC (newest first) + end + end + + def record_timestamp(record) + if record.is_a?(API::V3::Activities::ActivityEagerLoadingWrapper) + record.created_at&.to_i + elsif record.is_a?(Changeset) + record.committed_on.to_i + end + end +end diff --git a/spec/factories/work_package_factory.rb b/spec/factories/work_package_factory.rb index 0eb609fabd73..2dcafac0ce96 100644 --- a/spec/factories/work_package_factory.rb +++ b/spec/factories/work_package_factory.rb @@ -146,7 +146,6 @@ end end - # force done_ratio in status-based mode if given done_ratio is different from status default callback(:after_create) do |work_package, evaluator| next unless WorkPackage.status_based_mode? diff --git a/spec/services/work_packages/activities_tab/paginator_spec.rb b/spec/services/work_packages/activities_tab/paginator_spec.rb new file mode 100644 index 000000000000..583a7697c3ed --- /dev/null +++ b/spec/services/work_packages/activities_tab/paginator_spec.rb @@ -0,0 +1,253 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe WorkPackages::ActivitiesTab::Paginator do + shared_let(:user) { create(:admin) } + shared_let(:project) { create(:project) } + shared_let(:work_package) { create(:work_package, :created_in_past, project:, author: user, created_at: 4.days.ago) } + + let(:params) { {} } + let(:paginator) { described_class.new(work_package, params) } + + before do + allow(User).to receive(:current).and_return(user) + end + + describe "#call" do + context "with no additional journals" do + let(:work_package) { create(:work_package, project:, author: user) } + + it "returns paginated results with the initial journal" do + pagy, records = paginator.call + + expect(pagy).to be_a(Pagy) + expect(pagy.page).to eq(1) + expect(pagy.count).to eq(1) + expect(records).to have_attributes(size: 1) + expect(records.first).to be_a(API::V3::Activities::ActivityEagerLoadingWrapper) + end + end + + context "with multiple journals" do + 3.times do |i| + let!(:"journal_#{i + 1}") do + create(:work_package_journal, user:, notes: "Comment #{i + 1}", journable: work_package, version: i + 2) + end + end + + context "when user preference is set to asc sorting" do + before do + user.pref.update!(comments_sorting: :asc) + end + + it "returns journals reversed (oldest first)" do + _pagy, records = paginator.call + + expect(records.map(&:notes)).to eq(["", journal_1.notes, journal_2.notes, journal_3.notes]) + end + end + + context "when user preference is set to desc sorting" do + before do + user.pref.update!(comments_sorting: :desc) + end + + it "returns journals in DESC order (newest first)" do + _pagy, records = paginator.call + + expect(records.map(&:notes)).to eq([journal_3.notes, journal_2.notes, journal_1.notes, ""]) + end + end + end + + context "with changesets" do + let(:repository) { create(:repository_subversion, project:) } + + 2.times do |i| + let!(:"changeset_#{i + 1}") do + create(:changeset, + repository:, + committed_on: (2 - i).days.ago, # yesterday and today + revision: "rev#{i + 1}") + end + end + + before do + work_package.changesets << [changeset_1, changeset_2] + end + + it "includes changesets in the sorted results" do + _pagy, records = paginator.call + + expect(records.size).to eq(3) # 1 initial journal + 2 changesets + expect(records).to include(changeset_1, changeset_2) + end + + it "sorts changesets along with journals by timestamp" do + user.pref.update!(comments_sorting: :desc) + journal = create(:work_package_journal, + user:, + notes: "Comment between changesets", + journable: work_package, + version: work_package.journals.last.version + 1, + created_at: 1.5.days.ago) + + _pagy, records = paginator.call + expect(records).to eq([changeset_2, journal, changeset_1, work_package.journals.first]) + end + end + + context "with pagination" do + # Create enough journals to span multiple pages (default limit is 20) + 25.times do |i| + let!(:"journal_#{i + 1}") do + create(:work_package_journal, user:, notes: "Comment #{i + 1}", journable: work_package, version: i + 2) + end + end + + it "returns the first page with default limit" do + pagy, records = paginator.call + + expect(pagy.page).to eq(1) + expect(pagy.count).to eq(26) # 25 journals + 1 initial + expect(pagy.pages).to eq(2) + expect(records.size).to eq(20) # Default Pagy limit + end + + it "returns the second page when requested" do + params[:page] = 2 + pagy, records = paginator.call + + expect(pagy.page).to eq(2) + expect(records.size).to eq(6) # Remaining items + end + + context "with anchor to target journal" do + context "with comment anchor" do + it "returns the page containing the target journal" do + params[:anchor] = "comment-#{journal_1.id}" + pagy, records = paginator.call + + # journal_1 is old, so it should be on page 2 + expect(pagy.page).to eq(2) + expect(records.map(&:id)).to include(journal_1.id) + end + + it "handles invalid anchor format gracefully" do + params[:anchor] = "invalid-anchor" + pagy, _records = paginator.call + + expect(pagy.page).to eq(1) + end + + it "falls back to page 1 if journal not found" do + params[:anchor] = "comment-999999" + pagy, _records = paginator.call + + expect(pagy.page).to eq(1) + end + end + + context "with activity anchor" do + it "returns the page containing the target activity by sequence_version" do + params[:anchor] = "activity-2" + pagy, records = paginator.call + + # activity-2 corresponds to journal with sequence_version 2 + # which should be journal_1 + expect(pagy.page).to eq(2) + wrapped_journal = records.find { it.is_a?(API::V3::Activities::ActivityEagerLoadingWrapper) && it.id == journal_1.id } + expect(wrapped_journal.sequence_version).to eq(2) + end + + it "handles activity anchor for initial journal" do + params[:anchor] = "activity-1" + _pagy, records = paginator.call + + # activity-1 should be on the last page (oldest) + expect(records.any? { it.respond_to?(:sequence_version) && it.sequence_version == 1 }).to be(true) + end + end + end + end + + context "with internal comments filtering" do + let!(:internal_journal) do + create(:work_package_journal, + user:, + notes: "Internal comment", + journable: work_package, + internal: true, + version: 2) + end + let!(:public_journal) do + create(:work_package_journal, + user:, + notes: "Public comment", + journable: work_package, + internal: false, + version: 3) + end + + before do + work_package.project.enabled_internal_comments = true + work_package.project.save! + end + + context "when user can see internal comments" do + it "includes internal journals" do + _pagy, records = paginator.call + + journal_notes = records.map(&:notes) + expect(journal_notes).to include("Internal comment", "Public comment") + end + end + + context "when user cannot see internal comments" do + let(:member_role) { create(:project_role, permissions: %i[view_work_packages]) } + let(:member_user) { create(:user, member_with_roles: { project => member_role }) } + + before do + allow(User).to receive(:current).and_return(member_user) + end + + it "excludes internal journals" do + _pagy, records = described_class.new(work_package, params).call + + journal_notes = records.map(&:notes) + expect(journal_notes).not_to include("Internal comment") + expect(journal_notes).to include("Public comment") + end + end + end + end +end From 79404866ce2e628a9c412772848c09c5343228a1 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Wed, 8 Oct 2025 14:12:28 +0300 Subject: [PATCH 21/24] Add feature flag for activity tab lazy pagination --- .github/workflows/pullpreview.yml | 1 + .../activities_tab/index_component.html.erb | 63 +++++++----- .../activities_tab/index_component.rb | 22 +---- .../journals/index_component.html.erb | 85 ++++++++++++---- .../journals/index_component.rb | 94 +++++++++++++----- .../journals/lazy_index_component.html.erb | 29 ++++++ .../journals/lazy_index_component.rb | 78 +++++++++++++++ .../journals/lazy_page_component.html.erb | 2 +- .../activities_tab/lazy_index_component.rb | 51 ++++++++++ .../activities_tab_controller.rb | 99 ++++++++++++------- config/initializers/feature_decisions.rb | 3 + 11 files changed, 403 insertions(+), 124 deletions(-) create mode 100644 app/components/work_packages/activities_tab/journals/lazy_index_component.html.erb create mode 100644 app/components/work_packages/activities_tab/journals/lazy_index_component.rb create mode 100644 app/components/work_packages/activities_tab/lazy_index_component.rb diff --git a/.github/workflows/pullpreview.yml b/.github/workflows/pullpreview.yml index 04ec243c0a67..d0a565f99a39 100644 --- a/.github/workflows/pullpreview.yml +++ b/.github/workflows/pullpreview.yml @@ -37,6 +37,7 @@ jobs: echo "OPENPROJECT_ENTERPRISE__CHARGEBEE__SITE=openproject-enterprise-test" >> .env.pullpreview echo "OPENPROJECT_ENTERPRISE__TRIAL__CREATION__HOST=https://start.openproject-edge.com" >> .env.pullpreview echo "OPENPROJECT_FEATURE_BLOCK_NOTE_EDITOR=true" >> .env.pullpreview + echo "OPENPROJECT_FEATURE_WP_ACTIVITY_TAB_LAZY_PAGINATION_ACTIVE=true" >> .env.pullpreview - name: Boot as BIM edition if: contains(github.ref, 'bim/') || contains(github.head_ref, 'bim/') run: | diff --git a/app/components/work_packages/activities_tab/index_component.html.erb b/app/components/work_packages/activities_tab/index_component.html.erb index 73c5feb176b4..eb5a69537fb5 100644 --- a/app/components/work_packages/activities_tab/index_component.html.erb +++ b/app/components/work_packages/activities_tab/index_component.html.erb @@ -1,33 +1,48 @@ <%= - component_wrapper(tag: "turbo-frame") do - flex_layout(classes: "work-packages-activities-tab-index-component") do |activities_tab_wrapper_container| - activities_tab_wrapper_container.with_row(classes: "work-packages-activities-tab-index-component--errors") do - render(error_stream_component) - end - - activities_tab_wrapper_container.with_row(id: index_content_wrapper_key, data: wrapper_data_attributes) do - flex_layout do |activities_tab_container| - activities_tab_container.with_row(mb: 2) do - render(filter_and_sorting_component) - end + if deferred + render( + WorkPackages::ActivitiesTab::Journals::IndexComponent.new(work_package:, filter:, deferred:) + ) + else + component_wrapper(tag: "turbo-frame") do + flex_layout(classes: "work-packages-activities-tab-index-component") do |activities_tab_wrapper_container| + activities_tab_wrapper_container.with_row(classes: "work-packages-activities-tab-index-component--errors") do + render( + WorkPackages::ActivitiesTab::ErrorStreamComponent.new + ) + end - activities_tab_container.with_row(flex_layout: true, classes: "work-packages-activities-tab-index-component--content-container", mt: 3) do |journals_wrapper_container| - journals_wrapper_container.with_row( - classes: "work-packages-activities-tab-index-component--journals-container work-packages-activities-tab-index-component--journals-container_with-initial-input-compensation", - data: { "work-packages--activities-tab--index-target": "journalsContainer" } - ) do - render(list_journals_component) + activities_tab_wrapper_container.with_row(id: index_content_wrapper_key, data: wrapper_data_attributes) do + flex_layout do |activities_tab_container| + activities_tab_container.with_row(mb: 2) do + render( + WorkPackages::ActivitiesTab::Journals::FilterAndSortingComponent.new( + work_package:, + filter: + ) + ) end - if adding_comment_allowed? + activities_tab_container.with_row(flex_layout: true, classes: "work-packages-activities-tab-index-component--content-container", mt: 3) do |journals_wrapper_container| journals_wrapper_container.with_row( - id: add_comment_wrapper_key, - classes: "work-packages-activities-tab-index-component--input-container work-packages-activities-tab-index-component--input-container_sort-#{journal_sorting}", - p: 3, - bg: :subtle, - data: add_comment_wrapper_data_attributes + classes: "work-packages-activities-tab-index-component--journals-container work-packages-activities-tab-index-component--journals-container_with-initial-input-compensation", + data: { "work-packages--activities-tab--index-target": "journalsContainer" } ) do - render(add_journal_component) + render(list_journals_component) + end + + if adding_comment_allowed? + journals_wrapper_container.with_row( + id: add_comment_wrapper_key, + classes: "work-packages-activities-tab-index-component--input-container work-packages-activities-tab-index-component--input-container_sort-#{journal_sorting}", + p: 3, + bg: :subtle, + data: add_comment_wrapper_data_attributes + ) do + render( + WorkPackages::ActivitiesTab::Journals::NewComponent.new(work_package:, filter:, last_server_timestamp:) + ) + end end end end diff --git a/app/components/work_packages/activities_tab/index_component.rb b/app/components/work_packages/activities_tab/index_component.rb index e926d749b743..c07a0ff50c06 100644 --- a/app/components/work_packages/activities_tab/index_component.rb +++ b/app/components/work_packages/activities_tab/index_component.rb @@ -37,14 +37,13 @@ class IndexComponent < ApplicationComponent include WorkPackages::ActivitiesTab::SharedHelpers include WorkPackages::ActivitiesTab::StimulusControllers - def initialize(work_package:, journals:, paginator:, last_server_timestamp:, filter: :all) + def initialize(work_package:, last_server_timestamp:, filter: :all, deferred: false) super @work_package = work_package - @journals = journals - @paginator = paginator @filter = filter @last_server_timestamp = last_server_timestamp + @deferred = deferred end def self.wrapper_key = "work-package-activities-tab-content" @@ -52,26 +51,13 @@ def self.index_content_wrapper_key = WorkPackages::ActivitiesTab::StimulusContro def self.add_comment_wrapper_key = "work-packages-activities-tab-add-comment-component" delegate :index_content_wrapper_key, :add_comment_wrapper_key, to: :class - def filter_and_sorting_component - WorkPackages::ActivitiesTab::Journals::FilterAndSortingComponent.new(work_package:, filter:) - end - def list_journals_component - WorkPackages::ActivitiesTab::Journals::IndexComponent - .new(work_package:, journals:, filter:, paginator:) - end - - def add_journal_component - WorkPackages::ActivitiesTab::Journals::NewComponent.new(work_package:, filter:, last_server_timestamp:) - end - - def error_stream_component - WorkPackages::ActivitiesTab::ErrorStreamComponent.new + WorkPackages::ActivitiesTab::Journals::IndexComponent.new(work_package:, filter:) end private - attr_reader :work_package, :journals, :paginator, :filter, :last_server_timestamp + attr_reader :work_package, :filter, :last_server_timestamp, :deferred def wrapper_data_attributes # rubocop:disable Metrics/AbcSize stimulus_controllers = { diff --git a/app/components/work_packages/activities_tab/journals/index_component.html.erb b/app/components/work_packages/activities_tab/journals/index_component.html.erb index 763773a9fd8f..662bde4bd7aa 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/index_component.html.erb @@ -1,28 +1,75 @@ <%= - component_wrapper(class: "work-packages-activities-tab-journals-index-component") do - flex_layout(data: { test_selector: "op-wp-journals-#{filter}-#{journal_sorting}" }) do |journals_index_wrapper_container| - journals_index_wrapper_container.with_row( - classes: "work-packages-activities-tab-journals-index-component--journals-inner-container", - mb: inner_container_margin_bottom - ) do - flex_layout( - data: { test_selector: "op-wp-journals-container" } - ) do |journals_index_container| - if empty_state? - journals_index_container.with_row(mt: 2, mb: 3) do - render(WorkPackages::ActivitiesTab::Journals::EmptyComponent.new) - end - else - journals_index_container.with_row(id: insert_target_modifier_id) do - pages.map { concat render(it) } + if deferred + helpers.turbo_frame_tag("work-package-activities-tab-content-older-journals") do + flex_layout do |older_journals_container| + older_journals.each do |record| + older_journals_container.with_row do + if record.is_a?(Changeset) + render(WorkPackages::ActivitiesTab::Journals::RevisionComponent.new(changeset: record, filter:)) + else + render( + WorkPackages::ActivitiesTab::Journals::ItemComponent.new( + journal: record, filter:, + grouped_emoji_reactions: wp_journals_grouped_emoji_reactions[record.id] + ) + ) end end end end + end + else + component_wrapper(class: "work-packages-activities-tab-journals-index-component") do + flex_layout(data: { test_selector: "op-wp-journals-#{filter}-#{journal_sorting}" }) do |journals_index_wrapper_container| + journals_index_wrapper_container.with_row( + classes: "work-packages-activities-tab-journals-index-component--journals-inner-container", + mb: inner_container_margin_bottom + ) do + flex_layout( + id: insert_target_modifier_id, + data: { test_selector: "op-wp-journals-container" } + ) do |journals_index_container| + if empty_state? + journals_index_container.with_row(mt: 2, mb: 3) do + render( + WorkPackages::ActivitiesTab::Journals::EmptyComponent.new + ) + end + end + + if !journal_sorting_desc? && journals.count > MAX_RECENT_JOURNALS + journals_index_container.with_row do + helpers.turbo_frame_tag("work-package-activities-tab-content-older-journals", src: work_package_activities_path(work_package, filter:, deferred: true)) + end + end + + recent_journals.each do |record| + journals_index_container.with_row do + if record.is_a?(Changeset) + render(WorkPackages::ActivitiesTab::Journals::RevisionComponent.new(changeset: record, filter:)) + else + render( + WorkPackages::ActivitiesTab::Journals::ItemComponent.new( + journal: record, filter:, + grouped_emoji_reactions: wp_journals_grouped_emoji_reactions[record.id] + ) + ) + end + end + end + + if journal_sorting_desc? && journals.count > MAX_RECENT_JOURNALS + journals_index_container.with_row do + helpers.turbo_frame_tag("work-package-activities-tab-content-older-journals", src: work_package_activities_path(work_package, filter:, deferred: true)) + end + end + end + end - unless empty_state? - journals_index_wrapper_container - .with_row(classes: "work-packages-activities-tab-journals-index-component--stem-connection") + unless empty_state? + journals_index_wrapper_container + .with_row(classes: "work-packages-activities-tab-journals-index-component--stem-connection") + end end end end diff --git a/app/components/work_packages/activities_tab/journals/index_component.rb b/app/components/work_packages/activities_tab/journals/index_component.rb index 2bce3326e501..04fc8edb5d86 100644 --- a/app/components/work_packages/activities_tab/journals/index_component.rb +++ b/app/components/work_packages/activities_tab/journals/index_component.rb @@ -32,51 +32,89 @@ module WorkPackages module ActivitiesTab module Journals class IndexComponent < ApplicationComponent + MAX_RECENT_JOURNALS = 30 + include ApplicationHelper include OpPrimer::ComponentHelpers include OpTurbo::Streamable include WorkPackages::ActivitiesTab::SharedHelpers - def initialize(work_package:, journals:, paginator:, filter: :all) + def initialize(work_package:, filter: :all, deferred: false) super @work_package = work_package - @journals = journals - @paginator = paginator @filter = filter + @deferred = deferred end - def pages - current_page = paginator.page + private - result = (1..paginator.pages).map do |page| - if page == current_page - page_component(page) - else - lazy_page_component(page) - end - end + attr_reader :work_package, :filter, :deferred - result.tap { it.reverse! if journal_sorting.asc? && paginator.pages > 1 } + def insert_target_modified? + true end - def page_component(page) - WorkPackages::ActivitiesTab::Journals::PageComponent.new(journals:, emoji_reactions:, page:, filter:) + def insert_target_modifier_id + "work-package-journal-days" end - def lazy_page_component(page) - WorkPackages::ActivitiesTab::Journals::LazyPageComponent.new(work_package:, page:) + def journal_sorting_desc? + journal_sorting == "desc" end - def self.insert_target_modifier_id = "#{wrapper_key}-pages" - delegate :insert_target_modifier_id, to: :class + def base_journals + combine_and_sort_records(fetch_journals, fetch_revisions) + end - private + def fetch_journals + API::V3::Activities::ActivityEagerLoadingWrapper.wrap( + work_package + .journals + .internal_visible + .includes(:user, :customizable_journals, :attachable_journals, :storable_journals, :notifications) + .reorder(version: journal_sorting) + .with_sequence_version + ) + end - attr_reader :work_package, :journals, :paginator, :filter + def fetch_revisions + work_package.changesets.includes(:user, :repository) + end - def insert_target_modified? - true + def combine_and_sort_records(journals, revisions) + (journals + revisions).sort_by do |record| + timestamp = record_timestamp(record) + journal_sorting_desc? ? [-timestamp, -record.id] : [timestamp, record.id] + end + end + + def record_timestamp(record) + if record.is_a?(API::V3::Activities::ActivityEagerLoadingWrapper) + record.created_at&.to_i + elsif record.is_a?(Changeset) + record.committed_on.to_i + end + end + + def journals + base_journals + end + + def recent_journals + if journal_sorting_desc? + base_journals.first(MAX_RECENT_JOURNALS) + else + base_journals.last(MAX_RECENT_JOURNALS) + end + end + + def older_journals + if journal_sorting_desc? + base_journals.drop(MAX_RECENT_JOURNALS) + else + base_journals.take(base_journals.size - MAX_RECENT_JOURNALS) + end end def journal_with_notes @@ -85,8 +123,8 @@ def journal_with_notes .where.not(notes: "") end - def emoji_reactions - @emoji_reactions ||= + def wp_journals_grouped_emoji_reactions + @wp_journals_grouped_emoji_reactions ||= EmojiReactions::GroupedQueries.grouped_work_package_journals_emoji_reactions_by_reactable(work_package) end @@ -95,7 +133,11 @@ def empty_state? end def inner_container_margin_bottom - journal_sorting.desc? ? 3 : 0 + if journal_sorting_desc? + 3 + else + 0 + end end end end diff --git a/app/components/work_packages/activities_tab/journals/lazy_index_component.html.erb b/app/components/work_packages/activities_tab/journals/lazy_index_component.html.erb new file mode 100644 index 000000000000..763773a9fd8f --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/lazy_index_component.html.erb @@ -0,0 +1,29 @@ +<%= + component_wrapper(class: "work-packages-activities-tab-journals-index-component") do + flex_layout(data: { test_selector: "op-wp-journals-#{filter}-#{journal_sorting}" }) do |journals_index_wrapper_container| + journals_index_wrapper_container.with_row( + classes: "work-packages-activities-tab-journals-index-component--journals-inner-container", + mb: inner_container_margin_bottom + ) do + flex_layout( + data: { test_selector: "op-wp-journals-container" } + ) do |journals_index_container| + if empty_state? + journals_index_container.with_row(mt: 2, mb: 3) do + render(WorkPackages::ActivitiesTab::Journals::EmptyComponent.new) + end + else + journals_index_container.with_row(id: insert_target_modifier_id) do + pages.map { concat render(it) } + end + end + end + end + + unless empty_state? + journals_index_wrapper_container + .with_row(classes: "work-packages-activities-tab-journals-index-component--stem-connection") + end + end + end +%> diff --git a/app/components/work_packages/activities_tab/journals/lazy_index_component.rb b/app/components/work_packages/activities_tab/journals/lazy_index_component.rb new file mode 100644 index 000000000000..71bc21ca435b --- /dev/null +++ b/app/components/work_packages/activities_tab/journals/lazy_index_component.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +# ++ + +module WorkPackages + module ActivitiesTab + module Journals + class LazyIndexComponent < IndexComponent + def initialize(work_package:, journals:, paginator:, filter: :all) + super(work_package:, filter:, deferred: false) + + @journals = journals + @paginator = paginator + end + + def pages + current_page = paginator.page + + result = (1..paginator.pages).map do |page| + if page == current_page + page_component(page) + else + lazy_page_component(page) + end + end + + result.tap { it.reverse! if journal_sorting.asc? && paginator.pages > 1 } + end + + def page_component(page) + WorkPackages::ActivitiesTab::Journals::PageComponent + .new(journals:, emoji_reactions: wp_journals_grouped_emoji_reactions, page:, filter:) + end + + def lazy_page_component(page) + WorkPackages::ActivitiesTab::Journals::LazyPageComponent.new(work_package:, page:) + end + + def self.insert_target_modifier_id = "#{wrapper_key}-pages" + delegate :insert_target_modifier_id, to: :class + + private + + attr_reader :journals, :paginator + + def insert_target_modified? + true + end + end + end + end +end diff --git a/app/components/work_packages/activities_tab/journals/lazy_page_component.html.erb b/app/components/work_packages/activities_tab/journals/lazy_page_component.html.erb index f02e84f185af..cd0c1b0e348d 100644 --- a/app/components/work_packages/activities_tab/journals/lazy_page_component.html.erb +++ b/app/components/work_packages/activities_tab/journals/lazy_page_component.html.erb @@ -25,7 +25,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. See COPYRIGHT and LICENSE files for more details. -++#%> +++# %> <%= component_wrapper(data: wrapper_data_attributes) do diff --git a/app/components/work_packages/activities_tab/lazy_index_component.rb b/app/components/work_packages/activities_tab/lazy_index_component.rb new file mode 100644 index 000000000000..fc46e47a3692 --- /dev/null +++ b/app/components/work_packages/activities_tab/lazy_index_component.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +# ++ + +module WorkPackages + module ActivitiesTab + class LazyIndexComponent < IndexComponent + def initialize(work_package:, journals:, paginator:, last_server_timestamp:, filter: :all) + super(work_package:, last_server_timestamp:, filter:, deferred: false) + + @journals = journals + @paginator = paginator + end + + def list_journals_component + WorkPackages::ActivitiesTab::Journals::LazyIndexComponent + .new(work_package:, journals:, filter:, paginator:) + end + + private + + attr_reader :journals, :paginator + end + end +end diff --git a/app/controllers/work_packages/activities_tab_controller.rb b/app/controllers/work_packages/activities_tab_controller.rb index d92dbf5b3843..d8348e2ae0e6 100644 --- a/app/controllers/work_packages/activities_tab_controller.rb +++ b/app/controllers/work_packages/activities_tab_controller.rb @@ -39,19 +39,29 @@ class WorkPackages::ActivitiesTabController < ApplicationController before_action :find_journal, only: %i[edit cancel_edit update toggle_reaction] before_action :set_filter before_action :authorize - before_action :initialize_pagination, only: %i[index page_streams] + before_action :initialize_pagination, only: %i[page_streams] def index - render( - WorkPackages::ActivitiesTab::IndexComponent.new( - work_package: @work_package, - journals: @paginated_journals, - paginator: @paginator, - filter: @filter, - last_server_timestamp: get_current_server_timestamp - ), - layout: false - ) + index_component = + if OpenProject::FeatureDecisions.wp_activity_tab_lazy_pagination_active? + initialize_pagination + WorkPackages::ActivitiesTab::LazyIndexComponent.new( + work_package: @work_package, + journals: @paginated_journals, + paginator: @paginator, + filter: @filter, + last_server_timestamp: get_current_server_timestamp + ) + else + WorkPackages::ActivitiesTab::IndexComponent.new( + work_package: @work_package, + filter: @filter, + last_server_timestamp: get_current_server_timestamp, + deferred: ActiveRecord::Type::Boolean.new.cast(params[:deferred]) + ) + end + + render(index_component, layout: false) end def page_streams @@ -301,28 +311,37 @@ def handle_internal_server_error(error) end def replace_whole_tab - initialize_pagination # re-initialize pagination to pick up changes to sorting/filtering - replace_via_turbo_stream( - component: WorkPackages::ActivitiesTab::IndexComponent.new( - work_package: @work_package, - journals: @paginated_journals, - paginator: @paginator, - filter: @filter, - last_server_timestamp: get_current_server_timestamp - ) - ) + component = + if OpenProject::FeatureDecisions.wp_activity_tab_lazy_pagination_active? + initialize_pagination # re-initialize pagination to pick up changes to sorting/filtering + WorkPackages::ActivitiesTab::LazyIndexComponent.new( + work_package: @work_package, + journals: @paginated_journals, + paginator: @paginator, + filter: @filter, + last_server_timestamp: get_current_server_timestamp + ) + else + WorkPackages::ActivitiesTab::IndexComponent.new( + work_package: @work_package, + filter: @filter, + last_server_timestamp: get_current_server_timestamp + ) + end + replace_via_turbo_stream(component:) end def update_index_component - initialize_pagination # re-initialize pagination to pick up changes to sorting/filtering - update_via_turbo_stream( - component: WorkPackages::ActivitiesTab::Journals::IndexComponent.new( - work_package: @work_package, - journals: @paginated_journals, - paginator: @paginator, - filter: @filter - ) - ) + component = + if OpenProject::FeatureDecisions.wp_activity_tab_lazy_pagination_active? + initialize_pagination # re-initialize pagination to pick up changes to sorting/filtering + WorkPackages::ActivitiesTab::Journals::LazyIndexComponent + .new(work_package: @work_package, journals: @paginated_journals, paginator: @paginator, filter: @filter) + else + WorkPackages::ActivitiesTab::Journals::IndexComponent + .new(work_package: @work_package, filter: @filter) + end + update_via_turbo_stream(component:) end def create_journal_service_call @@ -426,12 +445,20 @@ def rerender_journals_with_updated_notification(journals, last_update_timestamp, end def insert_latest_journals_via_turbo_stream(journals, last_update_timestamp, emoji_reactions) - target_component = WorkPackages::ActivitiesTab::Journals::IndexComponent.new( - work_package: @work_package, - journals: Journal.none, # we do not need to pass any journals here since we just want the component key - paginator: nil, - filter: @filter - ) + target_component = + if OpenProject::FeatureDecisions.wp_activity_tab_lazy_pagination_active? + WorkPackages::ActivitiesTab::Journals::LazyIndexComponent.new( + work_package: @work_package, + journals: Journal.none, # we do not need to pass any journals here since we just want the component key + paginator: nil, + filter: @filter + ) + else + WorkPackages::ActivitiesTab::Journals::IndexComponent.new( + work_package: @work_package, + filter: @filter + ) + end journals.where("created_at > ?", last_update_timestamp).find_each do |journal| insert_via_turbo_stream( diff --git a/config/initializers/feature_decisions.rb b/config/initializers/feature_decisions.rb index 9f21912e6169..e5f187ff2cdd 100644 --- a/config/initializers/feature_decisions.rb +++ b/config/initializers/feature_decisions.rb @@ -75,3 +75,6 @@ OpenProject::FeatureDecisions.add :change_hierarchy_item_parent, description: "Enables a functionality to change the parent of a hierarchy item of " \ "custom fields of type hierarchy and scored list." + +OpenProject::FeatureDecisions.add :wp_activity_tab_lazy_pagination, + description: "Enables lazy pagination for the activity tab." From cfe3a2dbc8fd7ca94b826f623a6ac9dd8802df3f Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 9 Oct 2025 17:52:55 +0300 Subject: [PATCH 22/24] Group pagy with (legacy) will_paginate and remove version constraint --- Gemfile | 3 +-- Gemfile.lock | 6 +++--- config/initializers/pagy.rb | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Gemfile b/Gemfile index d0e7961c366b..76a0c4ac1be1 100644 --- a/Gemfile +++ b/Gemfile @@ -55,6 +55,7 @@ gem "request_store", "~> 1.7.0" gem "warden", "~> 1.2" gem "warden-basic_auth", "~> 0.2.1" +gem "pagy" gem "will_paginate", "~> 4.0.0" gem "friendly_id", "~> 5.5.0" @@ -420,5 +421,3 @@ end gem "openproject-octicons", "~>19.29.0" gem "openproject-octicons_helper", "~>19.29.0" gem "openproject-primer_view_components", "~>0.73.1" - -gem "pagy", "~> 9.3" diff --git a/Gemfile.lock b/Gemfile.lock index 7f307e058208..1d58120531c5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1119,7 +1119,7 @@ GEM ostruct (0.6.3) ox (2.14.23) bigdecimal (>= 3.0) - pagy (9.3.5) + pagy (9.4.0) paper_trail (16.0.0) activerecord (>= 6.1) request_store (~> 1.4) @@ -1711,7 +1711,7 @@ DEPENDENCIES opentelemetry-sdk (~> 1.9) overviews! ox - pagy (~> 9.3) + pagy paper_trail (~> 16.0.0) parallel_tests (~> 4.0) pdf-inspector (~> 1.2) @@ -2149,7 +2149,7 @@ CHECKSUMS ostruct (0.6.3) sha256=95a2ed4a4bd1d190784e666b47b2d3f078e4a9efda2fccf18f84ddc6538ed912 overviews (1.0.0) ox (2.14.23) sha256=4a9aedb4d6c78c5ebac1d7287dc7cc6808e14a8831d7adb727438f6a1b461b66 - pagy (9.3.5) sha256=78a9513150b96f872c092ab1cd95bb818ea29b2c417a4302290bc9293f8f0fd7 + pagy (9.4.0) sha256=db3f2e043f684155f18f78be62a81e8d033e39b9f97b1e1a8d12ad38d7bce738 paper_trail (16.0.0) sha256=e9b9f0fb1b8b590c8231cfa931b282ba92f90e066e393930a5e1c61ae4c5019d parallel (1.27.0) sha256=4ac151e1806b755fb4e2dc2332cbf0e54f2e24ba821ff2d3dcf86bf6dc4ae130 parallel_tests (4.10.1) sha256=df05458c691462b210f7a41fc2651d4e4e8a881e8190e6d1e122c92c07735d70 diff --git a/config/initializers/pagy.rb b/config/initializers/pagy.rb index d13c0b2f494d..b9901321dbda 100644 --- a/config/initializers/pagy.rb +++ b/config/initializers/pagy.rb @@ -105,7 +105,7 @@ # Keyset extra: Paginate with the Pagy keyset pagination technique # See https://ddnexus.github.io/pagy/docs/extras/keyset -require "pagy/extras/keyset" +# require "pagy/extras/keyset" # Meilisearch extra: Paginate `Meilisearch` result objects # See https://ddnexus.github.io/pagy/docs/extras/meilisearch From feb364e55eadd5a2a341b0b0c8adc7af9a489e72 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 9 Oct 2025 17:53:09 +0300 Subject: [PATCH 23/24] Allow pagy options in paginator for overrides in test --- .../work_packages/activities_tab/paginator.rb | 13 ++++++++---- .../activities_tab/paginator_spec.rb | 20 ++++++++++++------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/app/services/work_packages/activities_tab/paginator.rb b/app/services/work_packages/activities_tab/paginator.rb index eccb65d2dec8..21d1ee9ca5db 100644 --- a/app/services/work_packages/activities_tab/paginator.rb +++ b/app/services/work_packages/activities_tab/paginator.rb @@ -48,7 +48,7 @@ def call if anchor_type && target_record_id pagy_array_for_target_journal(anchor_type, target_record_id) else - pagy_array(base_journals) + pagy_array(base_journals, **pagy_options) end # For UI display: if user wants "oldest first" UI, reverse the array @@ -61,6 +61,10 @@ def call attr_reader :work_package, :params + def pagy_options + { page: params[:page] || 1, limit: params[:limit] || Pagy::DEFAULT[:limit], max_pages: 100 }.compact + end + def extract_target_record_id anchor = params[:anchor] # e.g., "comment-78758" (without #) return nil unless anchor @@ -83,11 +87,12 @@ def pagy_array_for_target_journal(anchor_type, target_record_id) end if target_index - target_page = (target_index / Pagy::DEFAULT[:limit]) + 1 - pagy_array(journals, page: target_page) + limit = pagy_options[:limit] + target_page = (target_index / limit) + 1 + pagy_array(journals, **pagy_options, page: target_page) else # Journal might be filtered out or deleted - fallback to page 1 - pagy_array(journals, page: 1) + pagy_array(journals, **pagy_options, page: 1) end end diff --git a/spec/services/work_packages/activities_tab/paginator_spec.rb b/spec/services/work_packages/activities_tab/paginator_spec.rb index 583a7697c3ed..cb2c72403896 100644 --- a/spec/services/work_packages/activities_tab/paginator_spec.rb +++ b/spec/services/work_packages/activities_tab/paginator_spec.rb @@ -127,20 +127,26 @@ end context "with pagination" do - # Create enough journals to span multiple pages (default limit is 20) - 25.times do |i| + # Create enough journals to span multiple pages (using limit of 5 for testing) + let(:test_limit) { 5 } + + 10.times do |i| let!(:"journal_#{i + 1}") do create(:work_package_journal, user:, notes: "Comment #{i + 1}", journable: work_package, version: i + 2) end end - it "returns the first page with default limit" do + before do + params[:limit] = test_limit + end + + it "returns the first page with specified limit" do pagy, records = paginator.call expect(pagy.page).to eq(1) - expect(pagy.count).to eq(26) # 25 journals + 1 initial - expect(pagy.pages).to eq(2) - expect(records.size).to eq(20) # Default Pagy limit + expect(pagy.count).to eq(11) # 10 journals + 1 initial + expect(pagy.pages).to eq(3) + expect(records.size).to eq(test_limit) end it "returns the second page when requested" do @@ -148,7 +154,7 @@ pagy, records = paginator.call expect(pagy.page).to eq(2) - expect(records.size).to eq(6) # Remaining items + expect(records.size).to eq(test_limit) end context "with anchor to target journal" do From 12a5f55b8887dd332393deff962ae3c10d88fb92 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Fri, 10 Oct 2025 10:10:31 +0300 Subject: [PATCH 24/24] Clarify scroll preservation behaviour based on UI layout sorting order --- .../activities-tab/lazy-page.controller.ts | 4 ++-- .../activities-tab/services/dom_helpers.ts | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts index 93dd0e50bfda..33e7c1dc3cbd 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/lazy-page.controller.ts @@ -102,8 +102,8 @@ export default class LazyPageController extends BaseController { const insertTargetId = this.insertTargetIdValue; if (scrollContainer && insertTargetId && stream.target.includes(insertTargetId)) { - const isPrepend = stream.action === 'prepend'; - void DomHelpers.keepScroll(scrollContainer, isPrepend, () => { + const isPrepending = this.indexOutlet.sortingAscending; // Newest at the bottom sorting order + void DomHelpers.keepScroll(scrollContainer, isPrepending, () => { event.detail.render(stream); return Promise.resolve(); }); diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts index b5cf04a90a16..485ed60ee746 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/services/dom_helpers.ts @@ -62,10 +62,10 @@ export namespace DomHelpers { * jump to a different part of the content. * * @param {HTMLElement} container - The scrollable container - * @param {boolean} top - Whether content is being added at the top (prepend) - * true: adjust scroll for prepended content - * false: maintain position for appended content - * @param {Function} fn - Async function that performs the DOM manipulation + * @param {boolean} isPrepending - Whether content is being added at the top (prepend) + * true: adjust scroll for prepended content + * false: maintain position for appended content + * @param {Function} renderFn - Async function that performs the DOM manipulation * * Algorithm: * - Capture current scroll position and total height BEFORE DOM changes @@ -75,15 +75,15 @@ export namespace DomHelpers { * For prepend: scrollTop + heightDifference (push view down by added content height) * For append: maintain original scrollTop (new content below doesn't affect view) */ - export async function keepScroll(container:HTMLElement, top:boolean, fn:() => Promise) { + export async function keepScroll(container:HTMLElement, isPrepending:boolean, renderFn:() => Promise) { pauseInertiaScroll(container); const scrollTop = container.scrollTop; const scrollHeight = container.scrollHeight; - await fn(); + await renderFn(); - if (top) { + if (isPrepending) { container.scrollTop = scrollTop + (container.scrollHeight - scrollHeight); } else { container.scrollTop = scrollTop;