From 9c8fba45c8473f37176a229fdd6ee528cd558c97 Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Wed, 20 Jul 2022 11:38:52 +0100 Subject: [PATCH 1/4] Stop using sticky JS on HTML publications - we've got a custom script on publications like /government/publications/heatwave-plan-for-england/beat-the-heat-staying-safe-in-hot-weather that keeps the contents list in the sidebar on the right as the page is scrolled - this script has a bug that lets the contents overlap the content at the bottom of the page in some situations - and we don't need the complexity of the JS now that CSS can do this natively - this isn't supported in Internet Explorer, but it fails gracefully --- .../stylesheets/views/_html-publication.scss | 5 +++++ .../content_items/html_publication.html.erb | 18 ++++-------------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/app/assets/stylesheets/views/_html-publication.scss b/app/assets/stylesheets/views/_html-publication.scss index e7390bcbb..0dc746624 100644 --- a/app/assets/stylesheets/views/_html-publication.scss +++ b/app/assets/stylesheets/views/_html-publication.scss @@ -22,6 +22,11 @@ } .contents-list-container { + @include govuk-media-query($from: desktop) { + position: sticky; + top: govuk-spacing(2); + } + .direction-rtl & { float: right; } diff --git a/app/views/content_items/html_publication.html.erb b/app/views/content_items/html_publication.html.erb index 496e0f2a9..64994598d 100644 --- a/app/views/content_items/html_publication.html.erb +++ b/app/views/content_items/html_publication.html.erb @@ -49,11 +49,7 @@ type: @content_item.schema_name } %> -
+
<% if @content_item.contents.any? %>
<%= render 'govuk_publishing_components/components/contents_list', contents: @content_item.contents, format_numbers: true %> @@ -75,14 +71,8 @@ <%= raw(@content_item.govspeak_body[:content]) %> <% end %>
+
-
- <%= render 'components/back-to-top', href: "#contents" %> -
- <%= render "govuk_publishing_components/components/print_link", { - margin_top: 0, - margin_bottom: 6, - } %> -
-
+
+ <%= render 'components/back-to-top', href: "#contents" %>
From 2fa2b95f53a208f4314ba1f9d8af3b1e4c83e744 Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Wed, 20 Jul 2022 11:50:48 +0100 Subject: [PATCH 2/4] Rework the page layout slightly - separate out the sidebar-with-body class from the grid row elements, so that the contents link at the bottom of the page is inside it and the margin bottom of sidebar-with-body comes beneath the contents link, not above it --- .../content_items/html_publication.html.erb | 42 ++++++++++--------- test/integration/html_publication_test.rb | 4 +- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/app/views/content_items/html_publication.html.erb b/app/views/content_items/html_publication.html.erb index 64994598d..35582323e 100644 --- a/app/views/content_items/html_publication.html.erb +++ b/app/views/content_items/html_publication.html.erb @@ -49,30 +49,32 @@ type: @content_item.schema_name } %> -
- <% if @content_item.contents.any? %> -
- <%= render 'govuk_publishing_components/components/contents_list', contents: @content_item.contents, format_numbers: true %> - <%= render "govuk_publishing_components/components/print_link", { - margin_top: 0, - margin_bottom: 6, - } %> +
+
+ <% if @content_item.contents.any? %> +
+ <%= render 'govuk_publishing_components/components/contents_list', contents: @content_item.contents, format_numbers: true %> + <%= render "govuk_publishing_components/components/print_link", { + margin_top: 0, + margin_bottom: 6, + } %> +
+ <% end %> + +
+
+ <%= render partial: "content_items/html_publication/print_meta_data" %> +
- <% end %> -
-
- <%= render partial: "content_items/html_publication/print_meta_data" %> +
+ <%= render "govuk_publishing_components/components/govspeak_html_publication", {} do %> + <%= raw(@content_item.govspeak_body[:content]) %> + <% end %>
-
- <%= render "govuk_publishing_components/components/govspeak_html_publication", {} do %> - <%= raw(@content_item.govspeak_body[:content]) %> - <% end %> +
+ <%= render 'components/back-to-top', href: "#contents" %>
- -
- <%= render 'components/back-to-top', href: "#contents" %> -
diff --git a/test/integration/html_publication_test.rb b/test/integration/html_publication_test.rb index 879e9c00e..df4f7a4b9 100644 --- a/test/integration/html_publication_test.rb +++ b/test/integration/html_publication_test.rb @@ -26,7 +26,7 @@ class HtmlPublicationTest < ActionDispatch::IntegrationTest test "html publications with meta data" do setup_and_visit_html_publication("print_with_meta_data") - within ".govuk-grid-row.sidebar-with-body" do + within ".sidebar-with-body" do assert page.find(".print-meta-data", visible: false) assert page.has_no_text?("© Crown copyright #{@content_item['details']['public_timestamp'].to_date.year}") @@ -37,7 +37,7 @@ class HtmlPublicationTest < ActionDispatch::IntegrationTest test "html publications with meta data - print version" do setup_and_visit_html_publication("print_with_meta_data", "?medium=print") - within ".govuk-grid-row.sidebar-with-body" do + within ".sidebar-with-body" do assert page.find(".print-meta-data", visible: true) assert page.has_text?("© Crown copyright #{@content_item['details']['public_timestamp'].to_date.year}") From 8ce0d0f83bb417f4c129c28b6a304e803fc444a7 Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Wed, 20 Jul 2022 15:37:32 +0100 Subject: [PATCH 3/4] Change jump link text - was 'contents' because it linked to the contents - however now the contents are sticky, can have this link and the contents on the same page, so 'Back to top' seems more appropriate - would remove altogether execept sticky isn't supported in IE so probably still useful to IE users --- app/views/content_items/html_publication.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/content_items/html_publication.html.erb b/app/views/content_items/html_publication.html.erb index 35582323e..622d08698 100644 --- a/app/views/content_items/html_publication.html.erb +++ b/app/views/content_items/html_publication.html.erb @@ -75,6 +75,6 @@
- <%= render 'components/back-to-top', href: "#contents" %> + <%= render 'components/back-to-top', href: "#contents", text: "Back to top" %>
From 608f30748f43493437f19e96bc2e24839a16ec22 Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Tue, 2 Aug 2022 11:16:25 +0100 Subject: [PATCH 4/4] Fix the tall menu problem - if the height of the contents list exceeds the height of the viewport users can't see the whole contents unless they scroll to the bottom of the page, where the sticky element hits the bottom and stops - to fix this, we apply a max height of the viewport and an overflow y scroll, so that the element never exceeds the height of the viewport and can be scrolled - also adding a bottom margin to prevent collision with the back to top link at the bottom --- app/assets/stylesheets/views/_html-publication.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/assets/stylesheets/views/_html-publication.scss b/app/assets/stylesheets/views/_html-publication.scss index 0dc746624..9d852a37c 100644 --- a/app/assets/stylesheets/views/_html-publication.scss +++ b/app/assets/stylesheets/views/_html-publication.scss @@ -25,6 +25,9 @@ @include govuk-media-query($from: desktop) { position: sticky; top: govuk-spacing(2); + max-height: 100vh; + overflow-y: auto; + margin-bottom: govuk-spacing(4); } .direction-rtl & {