From 5cc1f055f66c6ff7e9d9f0c339f12645f9292ace Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Wed, 4 Dec 2024 16:19:12 +0100 Subject: [PATCH 01/11] create a stimulus controller to handel scroll in the relations tab --- .../relations-tab/scroll.controller.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts new file mode 100644 index 000000000000..264732e1b6ee --- /dev/null +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts @@ -0,0 +1,17 @@ +import { Controller } from '@hotwired/stimulus'; + +export default class ScrollController extends Controller { + static values = { targetId: String }; + declare targetIdValue:string; + + connect() { + setTimeout(() => { + if (this.targetIdValue) { + const element = document.querySelector(`[data-test-selector='op-relation-row-${this.targetIdValue}']`) as HTMLElement; + if (element) { + element.scrollIntoView({ behavior: 'smooth', block: 'center' }); + } + } + }); + } +} From 57e2ab221a14c296cc79851d4187159c97d472c6 Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Wed, 4 Dec 2024 16:32:45 +0100 Subject: [PATCH 02/11] use scroll controller in wp relation index component --- .../work_package_relations_tab/index_component.html.erb | 6 +++++- .../work_package_relations_tab/index_component.rb | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/components/work_package_relations_tab/index_component.html.erb b/app/components/work_package_relations_tab/index_component.html.erb index e78e21510bbf..742eaca08200 100644 --- a/app/components/work_package_relations_tab/index_component.html.erb +++ b/app/components/work_package_relations_tab/index_component.html.erb @@ -53,7 +53,11 @@ %> <%= - flex_layout(mb: 3) do |flex| + flex_layout(mb: 3, data: { + "application-target": "dynamic", + controller: "work-packages--relations-tab--scroll", + "work-packages--relations-tab--scroll-target-id-value": @scroll_target_id, + }) do |flex| if any_relations? key_namespace = "#{I18N_NAMESPACE}.relations" diff --git a/app/components/work_package_relations_tab/index_component.rb b/app/components/work_package_relations_tab/index_component.rb index 083455043601..91426bf1ed58 100644 --- a/app/components/work_package_relations_tab/index_component.rb +++ b/app/components/work_package_relations_tab/index_component.rb @@ -11,13 +11,14 @@ class WorkPackageRelationsTab::IndexComponent < ApplicationComponent attr_reader :work_package, :relations, :children, :directionally_aware_grouped_relations - def initialize(work_package:, relations:, children:) + def initialize(work_package:, relations:, children:, scroll_target_id: nil) super() @work_package = work_package @relations = relations @children = children @directionally_aware_grouped_relations = group_relations_by_directional_context + @scroll_target_id = scroll_target_id end def self.wrapper_key From c15aef13b9c854fa0a3182a3baa6fbdd16706d92 Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Wed, 4 Dec 2024 16:35:12 +0100 Subject: [PATCH 03/11] pass target relation for scrolling in relation and children controllers while creating a new relation or child --- app/controllers/work_package_children_controller.rb | 3 ++- app/controllers/work_package_relations_controller.rb | 4 +++- .../dynamic/work-packages/relations-tab/scroll.controller.ts | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/work_package_children_controller.rb b/app/controllers/work_package_children_controller.rb index 2c95e3cc8a39..a302af0fc4e9 100644 --- a/app/controllers/work_package_children_controller.rb +++ b/app/controllers/work_package_children_controller.rb @@ -58,7 +58,8 @@ def create component = WorkPackageRelationsTab::IndexComponent.new( work_package: @work_package, relations: @relations, - children: @children + children: @children, + scroll_target_id: target_work_package_id ) replace_via_turbo_stream(component:) update_flash_message_via_turbo_stream( diff --git a/app/controllers/work_package_relations_controller.rb b/app/controllers/work_package_relations_controller.rb index 38095515fae5..6815f7c97e7e 100644 --- a/app/controllers/work_package_relations_controller.rb +++ b/app/controllers/work_package_relations_controller.rb @@ -60,10 +60,12 @@ def create .call(create_relation_params) if service_result.success? + target_work_package_id = params[:relation][:to_id] @work_package.reload component = WorkPackageRelationsTab::IndexComponent.new(work_package: @work_package, relations: @work_package.relations, - children: @work_package.children) + children: @work_package.children, + scroll_target_id: target_work_package_id) replace_via_turbo_stream(component:) respond_with_turbo_streams else diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts index 264732e1b6ee..4aa1aba25378 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts @@ -5,6 +5,7 @@ export default class ScrollController extends Controller { declare targetIdValue:string; connect() { + // Wait to ensure DOM is fully loaded setTimeout(() => { if (this.targetIdValue) { const element = document.querySelector(`[data-test-selector='op-relation-row-${this.targetIdValue}']`) as HTMLElement; From 87755749ff5c894bef4f52561402667070862864 Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Thu, 5 Dec 2024 12:15:55 +0100 Subject: [PATCH 04/11] fix failing test --- spec/controllers/work_package_relations_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/work_package_relations_controller_spec.rb b/spec/controllers/work_package_relations_controller_spec.rb index f6f8b36b2d84..2b8a63df5695 100644 --- a/spec/controllers/work_package_relations_controller_spec.rb +++ b/spec/controllers/work_package_relations_controller_spec.rb @@ -117,7 +117,7 @@ new_relation = Relation.last expect(WorkPackageRelationsTab::IndexComponent).to have_received(:new) - .with(work_package:, relations: [relation, new_relation], children:) + .with(work_package:, relations: [relation, new_relation], children:, scroll_target_id: unrelated_work_package.id) expect(controller).to have_received(:replace_via_turbo_stream) .with(component: an_instance_of(WorkPackageRelationsTab::IndexComponent)) end From 2c3485312b2c997e6eff82483d7ba11b44d031f8 Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Tue, 10 Dec 2024 17:22:40 +0100 Subject: [PATCH 05/11] remove settimeout from the controller and use requestAnimationFrame instead --- .../relations-tab/scroll.controller.ts | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts index 4aa1aba25378..76a6e6301843 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts @@ -1,18 +1,17 @@ import { Controller } from '@hotwired/stimulus'; export default class ScrollController extends Controller { - static values = { targetId: String }; - declare targetIdValue:string; - connect() { - // Wait to ensure DOM is fully loaded - setTimeout(() => { - if (this.targetIdValue) { - const element = document.querySelector(`[data-test-selector='op-relation-row-${this.targetIdValue}']`) as HTMLElement; - if (element) { - element.scrollIntoView({ behavior: 'smooth', block: 'center' }); - } - } - }); + this.waitForRenderAndAct(); + } + + waitForRenderAndAct() { + const element = document.querySelector('[data-scroll-active="true"]'); + + if (element) { + element.scrollIntoView({ behavior: 'smooth', block: 'center' }); + } else { + requestAnimationFrame(this.waitForRenderAndAct.bind(this)); + } } } From d00aea4eec26e45e89ad4c2af9dec60636ad28c3 Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Tue, 10 Dec 2024 17:25:51 +0100 Subject: [PATCH 06/11] change the attr name and set the selector on the row --- .../index_component.html.erb | 3 +-- .../index_component.rb | 18 ++++++++++++++---- .../work_package_children_controller.rb | 2 +- .../work_package_relations_controller.rb | 2 +- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/app/components/work_package_relations_tab/index_component.html.erb b/app/components/work_package_relations_tab/index_component.html.erb index 742eaca08200..cddd410e56be 100644 --- a/app/components/work_package_relations_tab/index_component.html.erb +++ b/app/components/work_package_relations_tab/index_component.html.erb @@ -55,8 +55,7 @@ <%= flex_layout(mb: 3, data: { "application-target": "dynamic", - controller: "work-packages--relations-tab--scroll", - "work-packages--relations-tab--scroll-target-id-value": @scroll_target_id, + controller: "work-packages--relations-tab--scroll" }) do |flex| if any_relations? key_namespace = "#{I18N_NAMESPACE}.relations" diff --git a/app/components/work_package_relations_tab/index_component.rb b/app/components/work_package_relations_tab/index_component.rb index 91426bf1ed58..4822e594b57b 100644 --- a/app/components/work_package_relations_tab/index_component.rb +++ b/app/components/work_package_relations_tab/index_component.rb @@ -9,16 +9,16 @@ class WorkPackageRelationsTab::IndexComponent < ApplicationComponent include Turbo::FramesHelper include OpTurbo::Streamable - attr_reader :work_package, :relations, :children, :directionally_aware_grouped_relations + attr_reader :work_package, :relations, :children, :directionally_aware_grouped_relations, :scroll_to_id - def initialize(work_package:, relations:, children:, scroll_target_id: nil) + def initialize(work_package:, relations:, children:, scroll_to_id: nil) super() @work_package = work_package @relations = relations @children = children @directionally_aware_grouped_relations = group_relations_by_directional_context - @scroll_target_id = scroll_target_id + @scroll_to_id = scroll_to_id end def self.wrapper_key @@ -62,7 +62,17 @@ def render_relation_group(title:, relation_type:, items:, &_block) end items.each do |item| - border_box.with_row(test_selector: row_test_selector(item)) do + related_work_package_id = if item.is_a?(Relation) + item.from_id == work_package.id ? item.to_id : item.from_id + else + item.id + end + + scroll_active = related_work_package_id.to_s == @scroll_to_id + border_box.with_row( + test_selector: row_test_selector(item), + data: { scroll_active: scroll_active } + ) do yield(item) end end diff --git a/app/controllers/work_package_children_controller.rb b/app/controllers/work_package_children_controller.rb index a302af0fc4e9..17cc55f95483 100644 --- a/app/controllers/work_package_children_controller.rb +++ b/app/controllers/work_package_children_controller.rb @@ -59,7 +59,7 @@ def create work_package: @work_package, relations: @relations, children: @children, - scroll_target_id: target_work_package_id + scroll_to_id: target_work_package_id ) replace_via_turbo_stream(component:) update_flash_message_via_turbo_stream( diff --git a/app/controllers/work_package_relations_controller.rb b/app/controllers/work_package_relations_controller.rb index 6815f7c97e7e..ca2b6bc06b4c 100644 --- a/app/controllers/work_package_relations_controller.rb +++ b/app/controllers/work_package_relations_controller.rb @@ -65,7 +65,7 @@ def create component = WorkPackageRelationsTab::IndexComponent.new(work_package: @work_package, relations: @work_package.relations, children: @work_package.children, - scroll_target_id: target_work_package_id) + scroll_to_id: target_work_package_id) replace_via_turbo_stream(component:) respond_with_turbo_streams else From e8c63b0bf55898144dd585200975bb90fbc5b6cd Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Tue, 10 Dec 2024 17:31:54 +0100 Subject: [PATCH 07/11] change scroll active to scroll to --- app/components/work_package_relations_tab/index_component.rb | 4 ++-- .../dynamic/work-packages/relations-tab/scroll.controller.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/components/work_package_relations_tab/index_component.rb b/app/components/work_package_relations_tab/index_component.rb index 4822e594b57b..46993884cd32 100644 --- a/app/components/work_package_relations_tab/index_component.rb +++ b/app/components/work_package_relations_tab/index_component.rb @@ -68,10 +68,10 @@ def render_relation_group(title:, relation_type:, items:, &_block) item.id end - scroll_active = related_work_package_id.to_s == @scroll_to_id + scroll_to = related_work_package_id.to_s == @scroll_to_id border_box.with_row( test_selector: row_test_selector(item), - data: { scroll_active: scroll_active } + data: { scroll_to: scroll_to } ) do yield(item) end diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts index 76a6e6301843..3c05bc1753ff 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts @@ -6,7 +6,7 @@ export default class ScrollController extends Controller { } waitForRenderAndAct() { - const element = document.querySelector('[data-scroll-active="true"]'); + const element = document.querySelector('[data-scroll-to="true"]'); if (element) { element.scrollIntoView({ behavior: 'smooth', block: 'center' }); From e3d54b4159f1b11f9510778b27d3cd3006d7239d Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Tue, 10 Dec 2024 17:53:05 +0100 Subject: [PATCH 08/11] fix rubocup error --- app/components/work_package_relations_tab/index_component.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/components/work_package_relations_tab/index_component.rb b/app/components/work_package_relations_tab/index_component.rb index 46993884cd32..551f87432b2a 100644 --- a/app/components/work_package_relations_tab/index_component.rb +++ b/app/components/work_package_relations_tab/index_component.rb @@ -67,11 +67,9 @@ def render_relation_group(title:, relation_type:, items:, &_block) else item.id end - - scroll_to = related_work_package_id.to_s == @scroll_to_id border_box.with_row( test_selector: row_test_selector(item), - data: { scroll_to: scroll_to } + data: { scroll_to: related_work_package_id.to_s == @scroll_to_id } ) do yield(item) end From 74abdc80f2ec848a9f0538d58f59026322005844 Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Tue, 10 Dec 2024 18:03:46 +0100 Subject: [PATCH 09/11] fix failing feature test --- .../index_component.rb | 18 +++++++++--------- .../work_package_relations_controller_spec.rb | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/components/work_package_relations_tab/index_component.rb b/app/components/work_package_relations_tab/index_component.rb index 551f87432b2a..3a6491870b99 100644 --- a/app/components/work_package_relations_tab/index_component.rb +++ b/app/components/work_package_relations_tab/index_component.rb @@ -62,11 +62,7 @@ def render_relation_group(title:, relation_type:, items:, &_block) end items.each do |item| - related_work_package_id = if item.is_a?(Relation) - item.from_id == work_package.id ? item.to_id : item.from_id - else - item.id - end + related_work_package_id = find_related_work_package_id(item) border_box.with_row( test_selector: row_test_selector(item), data: { scroll_to: related_work_package_id.to_s == @scroll_to_id } @@ -92,11 +88,15 @@ def new_button_test_selector(relation_type:) end def row_test_selector(item) + related_work_package_id = find_related_work_package_id(item) + "op-relation-row-#{related_work_package_id}" + end + + def find_related_work_package_id(item) if item.is_a?(Relation) - target = item.to == work_package ? item.from : item.to - "op-relation-row-#{target.id}" - else # Work Package object - "op-relation-row-#{item.id}" + item.from_id == work_package.id ? item.to_id : item.from_id + else + item.id end end end diff --git a/spec/controllers/work_package_relations_controller_spec.rb b/spec/controllers/work_package_relations_controller_spec.rb index 2b8a63df5695..3c617af90b4c 100644 --- a/spec/controllers/work_package_relations_controller_spec.rb +++ b/spec/controllers/work_package_relations_controller_spec.rb @@ -117,7 +117,7 @@ new_relation = Relation.last expect(WorkPackageRelationsTab::IndexComponent).to have_received(:new) - .with(work_package:, relations: [relation, new_relation], children:, scroll_target_id: unrelated_work_package.id) + .with(work_package:, relations: [relation, new_relation], children:, scroll_to_id: unrelated_work_package.id) expect(controller).to have_received(:replace_via_turbo_stream) .with(component: an_instance_of(WorkPackageRelationsTab::IndexComponent)) end From 12264c48271065048324dff5a871c09069156dcc Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Tue, 10 Dec 2024 18:41:22 +0100 Subject: [PATCH 10/11] fix failing rubocup errors --- .../index_component.rb | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/app/components/work_package_relations_tab/index_component.rb b/app/components/work_package_relations_tab/index_component.rb index 3a6491870b99..d30e3191ba63 100644 --- a/app/components/work_package_relations_tab/index_component.rb +++ b/app/components/work_package_relations_tab/index_component.rb @@ -48,27 +48,36 @@ def group_relations_by_directional_context def any_relations? = relations.any? || children.any? def render_relation_group(title:, relation_type:, items:, &_block) - render(border_box_container(padding: :condensed, - data: { test_selector: "op-relation-group-#{relation_type}" })) do |border_box| - border_box.with_header(py: 3) do - flex_layout(align_items: :center) do |flex| - flex.with_column(mr: 2) do - render(Primer::Beta::Text.new(font_size: :normal, font_weight: :bold)) { title } - end - flex.with_column do - render(Primer::Beta::Counter.new(count: items.size, round: true, scheme: :primary)) - end + render(border_box_container( + padding: :condensed, + data: { test_selector: "op-relation-group-#{relation_type}" } + )) do |border_box| + render_header(border_box, title, items) + render_items(border_box, items, &_block) + end + end + + def render_header(border_box, title, items) + border_box.with_header(py: 3) do + flex_layout(align_items: :center) do |flex| + flex.with_column(mr: 2) do + render(Primer::Beta::Text.new(font_size: :normal, font_weight: :bold)) { title } + end + flex.with_column do + render(Primer::Beta::Counter.new(count: items.size, round: true, scheme: :primary)) end end + end + end - items.each do |item| - related_work_package_id = find_related_work_package_id(item) - border_box.with_row( - test_selector: row_test_selector(item), - data: { scroll_to: related_work_package_id.to_s == @scroll_to_id } - ) do - yield(item) - end + def render_items(border_box, items) + items.each do |item| + related_work_package_id = find_related_work_package_id(item) + border_box.with_row( + test_selector: row_test_selector(item), + data: { scroll_to: related_work_package_id.to_s == @scroll_to_id } + ) do + yield(item) end end end From d878858721d349bbeefc757bdba2faa5a649d1dc Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Mon, 16 Dec 2024 13:17:36 +0100 Subject: [PATCH 11/11] use target in stimulus controller --- .../index_component.html.erb | 5 +---- .../index_component.rb | 10 +++++++++- .../relations-tab/scroll.controller.ts | 17 ++++++++--------- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/app/components/work_package_relations_tab/index_component.html.erb b/app/components/work_package_relations_tab/index_component.html.erb index cddd410e56be..e78e21510bbf 100644 --- a/app/components/work_package_relations_tab/index_component.html.erb +++ b/app/components/work_package_relations_tab/index_component.html.erb @@ -53,10 +53,7 @@ %> <%= - flex_layout(mb: 3, data: { - "application-target": "dynamic", - controller: "work-packages--relations-tab--scroll" - }) do |flex| + flex_layout(mb: 3) do |flex| if any_relations? key_namespace = "#{I18N_NAMESPACE}.relations" diff --git a/app/components/work_package_relations_tab/index_component.rb b/app/components/work_package_relations_tab/index_component.rb index d30e3191ba63..b64aac1cf479 100644 --- a/app/components/work_package_relations_tab/index_component.rb +++ b/app/components/work_package_relations_tab/index_component.rb @@ -73,9 +73,17 @@ def render_header(border_box, title, items) def render_items(border_box, items) items.each do |item| related_work_package_id = find_related_work_package_id(item) + data_attribute = nil + if related_work_package_id.to_s == @scroll_to_id + data_attribute = { + controller: "work-packages--relations-tab--scroll", + application_target: "dynamic", + "work-packages--relations-tab--scroll-target": "scrollToRow" + } + end border_box.with_row( test_selector: row_test_selector(item), - data: { scroll_to: related_work_package_id.to_s == @scroll_to_id } + data: data_attribute ) do yield(item) end diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts index 3c05bc1753ff..ebf2b4eb5d16 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/relations-tab/scroll.controller.ts @@ -1,17 +1,16 @@ import { Controller } from '@hotwired/stimulus'; export default class ScrollController extends Controller { - connect() { - this.waitForRenderAndAct(); - } + static targets = ['scrollToRow']; - waitForRenderAndAct() { - const element = document.querySelector('[data-scroll-to="true"]'); + declare readonly scrollToRowTarget:HTMLElement; + declare readonly hasScrollToRowTarget:boolean; - if (element) { - element.scrollIntoView({ behavior: 'smooth', block: 'center' }); - } else { - requestAnimationFrame(this.waitForRenderAndAct.bind(this)); + scrollToRowTargetConnected() { + if (this.hasScrollToRowTarget) { + setTimeout(() => { + this.scrollToRowTarget.scrollIntoView({ behavior: 'smooth', block: 'center' }); + }); } } }