From 11761dda5c1ac9b27c1d821dbce0a859203abdb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 6 Mar 2020 01:04:55 +0000 Subject: [PATCH 1/4] Restore the CWM::RichText scroll after updating its content --- library/cwm/src/lib/cwm/common_widgets.rb | 34 +++++++++++++++++ library/cwm/test/common_widgets_test.rb | 45 ++++++++++++++++++++++- 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/library/cwm/src/lib/cwm/common_widgets.rb b/library/cwm/src/lib/cwm/common_widgets.rb index e63357c20..ac7838037 100644 --- a/library/cwm/src/lib/cwm/common_widgets.rb +++ b/library/cwm/src/lib/cwm/common_widgets.rb @@ -365,6 +365,40 @@ class RichText < AbstractWidget self.widget_type = :richtext include ValueBasedWidget + + # Determines if the scroll should be restored after updating the content + # + # @return [Boolean] true if the scroll should be restored; false otherwise + def keep_scroll? + false + end + + # Updates the content + # + # Depending on #keep_scroll?, the scroll will be saved and restored. + # + # @param val [String] the new content for the widget + def value=(val) + current_vscroll = vscroll + super + self.vscroll = current_vscroll if keep_scroll? + end + + private + + # Saves the current vertical scroll + # + # @return [String] current vertical scroll value + def vscroll + Yast::UI.QueryWidget(Id(widget_id), :VScrollValue) + end + + # Sets vertical scroll + # + # @param value [String] the new vertical scroll value + def vscroll=(value) + Yast::UI.ChangeWidget(Id(widget_id), :VScrollValue, value) + end end # Time field widget diff --git a/library/cwm/test/common_widgets_test.rb b/library/cwm/test/common_widgets_test.rb index 9d1c9a2d1..7d6a42a7c 100755 --- a/library/cwm/test/common_widgets_test.rb +++ b/library/cwm/test/common_widgets_test.rb @@ -6,7 +6,6 @@ require "cwm/rspec" describe CWM::RadioButtons do - class TestRadioButtons < CWM::RadioButtons def label "Choose a number" @@ -59,3 +58,47 @@ def hspacing end end end + +describe CWM::RichText do + subject { described_class.new } + let(:widget_id) { Id(subject.widget_id) } + + describe "#value=" do + before do + allow(subject).to receive(:keep_scroll?).and_return(keep_scroll) + allow(Yast::UI).to receive(:ChangeWidget) + end + + context "when set to restore the scroll" do + let(:keep_scroll) { true } + + it "saves the scroll position" do + expect(Yast::UI).to receive(:QueryWidget).with(widget_id, :VScrollValue) + + subject.value = "Test" + end + + it "restores the scroll" do + expect(Yast::UI).to receive(:ChangeWidget).with(widget_id, :VScrollValue, anything) + + subject.value = "Test" + end + end + + context "when set to not restore the scroll" do + let(:keep_scroll) { false } + + it "saves the scroll position" do + expect(Yast::UI).to receive(:QueryWidget).with(widget_id, :VScrollValue) + + subject.value = "Test" + end + + it "does not restore the scroll" do + expect(Yast::UI).to_not receive(:ChangeWidget).with(widget_id, :VScrollValue, anything) + + subject.value = "Test" + end + end + end +end From 9ceafe183866143209dd396fd3d80474f3a6ef98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 6 Mar 2020 13:29:34 +0000 Subject: [PATCH 2/4] Bump version and update changelog --- package/yast2.changes | 7 +++++++ package/yast2.spec | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/package/yast2.changes b/package/yast2.changes index fb70fe456..6ebb7929a 100644 --- a/package/yast2.changes +++ b/package/yast2.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Fri Mar 6 13:26:57 UTC 2020 - David Diaz + +- Allow to restore the vertical scroll of a CWM::RichText + (related to bsc#1049965) +- 4.2.70 + ------------------------------------------------------------------- Fri Mar 6 12:00:43 UTC 2020 - David Diaz diff --git a/package/yast2.spec b/package/yast2.spec index a3b4eba6f..3e3343c43 100644 --- a/package/yast2.spec +++ b/package/yast2.spec @@ -17,7 +17,7 @@ Name: yast2 -Version: 4.2.69 +Version: 4.2.70 Release: 0 Summary: YaST2 Main Package License: GPL-2.0-only From d5e1be8ca1968e833800bb52690dc8700cd715b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 9 Mar 2020 08:48:54 +0000 Subject: [PATCH 3/4] Update from code review --- library/cwm/src/lib/cwm/common_widgets.rb | 6 ++-- library/cwm/test/common_widgets_test.rb | 38 ++++++++++++++--------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/library/cwm/src/lib/cwm/common_widgets.rb b/library/cwm/src/lib/cwm/common_widgets.rb index ac7838037..a6c7c8ab0 100644 --- a/library/cwm/src/lib/cwm/common_widgets.rb +++ b/library/cwm/src/lib/cwm/common_widgets.rb @@ -366,16 +366,16 @@ class RichText < AbstractWidget include ValueBasedWidget - # Determines if the scroll should be restored after updating the content + # Determines if the vertical scroll must be kept after updating the content # - # @return [Boolean] true if the scroll should be restored; false otherwise + # @return [Boolean] true if the vertical scroll must be kept; false otherwise def keep_scroll? false end # Updates the content # - # Depending on #keep_scroll?, the scroll will be saved and restored. + # Depending on #keep_scroll?, the vertical scroll will be saved and restored. # # @param val [String] the new content for the widget def value=(val) diff --git a/library/cwm/test/common_widgets_test.rb b/library/cwm/test/common_widgets_test.rb index 7d6a42a7c..4fa68366c 100755 --- a/library/cwm/test/common_widgets_test.rb +++ b/library/cwm/test/common_widgets_test.rb @@ -62,42 +62,50 @@ def hspacing describe CWM::RichText do subject { described_class.new } let(:widget_id) { Id(subject.widget_id) } + let(:new_content) { "Updated content" } + let(:vscroll) { "40" } describe "#value=" do before do - allow(subject).to receive(:keep_scroll?).and_return(keep_scroll) + allow(Yast::UI).to receive(:QueryWidget).with(widget_id, :VScrollValue).and_return(vscroll) allow(Yast::UI).to receive(:ChangeWidget) end - context "when set to restore the scroll" do - let(:keep_scroll) { true } + context "when set to keep the scroll" do + before do + allow(subject).to receive(:keep_scroll?).and_return(true) + end + + it "changes the widget value" do + expect(Yast::UI).to receive(:ChangeWidget).with(widget_id, :Value, new_content) + + subject.value = new_content + end - it "saves the scroll position" do + it "saves vertical scroll position" do expect(Yast::UI).to receive(:QueryWidget).with(widget_id, :VScrollValue) - subject.value = "Test" + subject.value = new_content end - it "restores the scroll" do - expect(Yast::UI).to receive(:ChangeWidget).with(widget_id, :VScrollValue, anything) + it "restores vertical scroll" do + expect(Yast::UI).to receive(:ChangeWidget).with(widget_id, :VScrollValue, vscroll) - subject.value = "Test" + subject.value = new_content end end - context "when set to not restore the scroll" do - let(:keep_scroll) { false } - - it "saves the scroll position" do - expect(Yast::UI).to receive(:QueryWidget).with(widget_id, :VScrollValue) + context "when set to not keep the scroll" do + it "changes the widget value" do + expect(Yast::UI).to receive(:ChangeWidget).with(widget_id, :Value, new_content) - subject.value = "Test" + subject.value = new_content end it "does not restore the scroll" do expect(Yast::UI).to_not receive(:ChangeWidget).with(widget_id, :VScrollValue, anything) - subject.value = "Test" + subject.value = new_content end end end From 6167ebbbd7496a109fe954edb41ab677ce59579c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 9 Mar 2020 09:53:47 +0000 Subject: [PATCH 4/4] Improve the documentation --- library/cwm/src/lib/cwm/common_widgets.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/library/cwm/src/lib/cwm/common_widgets.rb b/library/cwm/src/lib/cwm/common_widgets.rb index a6c7c8ab0..eb251c87e 100644 --- a/library/cwm/src/lib/cwm/common_widgets.rb +++ b/library/cwm/src/lib/cwm/common_widgets.rb @@ -368,6 +368,13 @@ class RichText < AbstractWidget # Determines if the vertical scroll must be kept after updating the content # + # @note Useful only to keep the sense of continuity when redrawing basically with the same text + # + # Keeping the vertical scroll after changing the value is mostly intended to be used after a + # redraw because of a user action. However, using it after changing the content noticeably + # (e.g., displaying different product descriptions), will look like a randomly positioned + # vertical scroll. + # # @return [Boolean] true if the vertical scroll must be kept; false otherwise def keep_scroll? false