Display contents lists under specific conditions #719

emmabeynon · 2018-01-23T11:03:10Z

For https://trello.com/c/2UIR4Sen/210-investigate-how-to-deal-with-short-content-lists

We have identified that there are occasions where it's not appropriate to display a contents list, for example when there is only one item on the list. Having spiked an approach for displaying a contents list dependent on certain conditions, this PR implements the approach. We've reached the below criteria for displaying a contents list in conjunction with Si Stephens-Manassiev in the content team:

We won't show a contents list that has one item.
We will show a contents list with 2 items if the first item's character count is over 415.
We will show a contents list with 2 items if the first item contains a table with more than 13 rows.
We will show a contents list with 2 items if the first item contains an image and character count is over 224.
We will show a contents list with 2 items if the first item contains an image and a table with more than 6 rows.
We will show a contents list with 3 items or more.

To Do

Update detailed guides after Move detailed guides to new layout #706 has been merged.

Component guide for this PR:
https://government-frontend-pr-719.herokuapp.com/component-guide

fofr · 2018-01-23T11:14:14Z

app/presenters/content_item/contents_list.rb

+    end
+
+    def first_item_content
+      element = Nokogiri::HTML(body).css('p').first


Some questions about this part:

what happens if the content begins with something that's not a paragraph, eg a table or image?

is this relatively slow at the moment? how performant could we make it?

Good point about the paragraphs - I'll try and find some Detailed Guides examples with other content and work something out.
Steve's got a suggestion for optimising the Nokogiri stuff below 👇

fofr · 2018-01-23T11:15:16Z

app/presenters/content_item/contents_list.rb

@@ -3,6 +3,7 @@ module ContentsList
    include ActionView::Helpers::UrlHelper
    include TypographyHelper

+    WORD_COUNT_LIMIT = 100


I wonder if total word count is a useful heuristic.

What @fofr said. Some formats will contain medical terminology with superlongtechnicalwords, some will contain more legalese. I think a char count might be a more honest metric.

fofr · 2018-01-23T11:16:21Z

Love this as a starting point.

Are there some example URLs showing the effect?

steventux · 2018-01-23T11:33:02Z

app/presenters/content_item/contents_list.rb

+    end
+
+    def first_item_content
+      element = Nokogiri::HTML(body).css('p').first


I think we can optimise this a bit by only parsing the body once with Nokogiri, building the contents list as the existing implementation does, but also populating the first n chapters so we can check their length.
This https://stackoverflow.com/questions/3484874/how-to-split-a-html-document-using-nokogiri does something similar by building a hash of chapters.

steventux · 2018-01-23T11:34:41Z

test/integration/topical_event_about_page_test.rb

+
+  def long_first_item_body
+    "<div class='govspeak'><h2 id='response-in-the-uk'>Item 1</h2>
+    <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean facilisis


The Faker gem is great for generating this sort of thing.

Nice one! Will keep that in mind for production tests 👍

We can also use the govuk_schema gem to generate random examples to test this with

emmabeynon · 2018-01-23T11:52:37Z

Are there some example URLs showing the effect?

We've got this as an example of the contents list not showing if there's only one item: https://government-frontend-pr-719.herokuapp.com/government/topical-events/ukraine-reform-conference/about
Unfortunately there aren't any live examples of Topical Event About Pages with two items - I've fudged some data locally to check it works - but I know that Detailed Guides has some, so I can extend this to the view for this format and will update the PR with an example 👍
And then I remembered that Detailed Guides hasn't been moved to universal layout yet 🤦‍♀️

steventux · 2018-01-23T11:56:00Z

app/views/content_items/topical_event_about_page.html.erb

-    <%= render "components/contents-list-with-body", {
-      contents: @content_item.contents
-    } do %>
+    <% if @content_item.show_content_list? %>


This might be better implemented in the contents-list-with-body component as there's already a check for @content_item.contents.any? which prevents the list and the contents link from displaying in the absence of any contents.

Yeah definitely - I've noted it's not an ideal implementation in the PR description, and would need to investigate other formats using this component before making changes to it in prod.

emmabeynon · 2018-01-24T14:24:55Z

@fofr @steventux I've added a condition for showing a contents list when a table has a certain number of rows, and a condition for when an image is present (I imagine we could use the same idea for videos). Regarding images, I had a quick chat with Steve about this last night and we agreed that since it seems that images are always the same size, the simplest way to deal with them is to just show contents lists when they're present.

steventux · 2018-01-24T14:51:00Z

app/presenters/content_item/contents_list.rb

+      return unless table.present?
+
+      row_count = table.css('tr').count
+      row_count > 10


I know this PR is still quite experimental, so a reminder to use the relevant constant TABLE_ROW_LIMIT here.

Yep, that'll be me messing around with row numbers and not replacing the constant 🤦‍♀️

steventux · 2018-01-24T15:00:10Z

app/presenters/content_item/contents_list.rb

+      row_count > 10
+    end
+
+    def find_table


This looks very similar to the following method, wonder if the logic is similar enough for one method.

In order to generate lorem ipsum for longer pieces of content in tests.

We have found instances where displaying a contents list on a page isn't always appropriate, such as when there is only one item in the list. We have decided to not show the contents list if there are fewer than three items, with an additional condition that we do show the contents list if the first item's content is more than 100 characters.

We now show the contents list in cases where there are only two content items if the first item contains a table with more than 13 rows.

We now show the contents list when there are 2 items on a contents lists and the first item has both an image and content over 50 characters.

We now show the contents list when there are 2 items on a contents lists and the first item has both an image and a table with more than 6 rows.

We update test coverage to reflect that contents lists are shown under certain conditions.

We update test coverage to reflect that contents lists are shown under certain conditions. We have found that Working Groups with policies don't always have a body, which caused problems in the current code implementation when it came to checking content length in the first item of a contents list. Therefore we now check if a "first item" is present in the body before carrying out the content checks.

We update test coverage to reflect that contents lists are shown under certain conditions.

tijmenb temporarily deployed to government-frontend-pr-719 January 23, 2018 11:03 Inactive

emmabeynon requested review from fofr and steventux January 23, 2018 11:03

fofr reviewed Jan 23, 2018

View reviewed changes

steventux reviewed Jan 23, 2018

View reviewed changes

emmabeynon force-pushed the spike-content-list-length branch from e121f0e to 1a739c7 Compare January 23, 2018 16:42

boffbowsh temporarily deployed to government-frontend-pr-719 January 23, 2018 16:42 Inactive

emmabeynon force-pushed the spike-content-list-length branch from 1a739c7 to 96338e2 Compare January 23, 2018 16:45

boffbowsh temporarily deployed to government-frontend-pr-719 January 23, 2018 16:45 Inactive

emmabeynon force-pushed the spike-content-list-length branch from 96338e2 to f359951 Compare January 24, 2018 09:40

boffbowsh temporarily deployed to government-frontend-pr-719 January 24, 2018 09:41 Inactive

emmabeynon force-pushed the spike-content-list-length branch from f359951 to d8cc89d Compare January 24, 2018 11:20

boffbowsh temporarily deployed to government-frontend-pr-719 January 24, 2018 11:20 Inactive

emmabeynon force-pushed the spike-content-list-length branch from d8cc89d to e8f642f Compare January 24, 2018 11:38

boffbowsh temporarily deployed to government-frontend-pr-719 January 24, 2018 11:38 Inactive

emmabeynon force-pushed the spike-content-list-length branch from e8f642f to 7ef77a5 Compare January 24, 2018 14:38

boffbowsh temporarily deployed to government-frontend-pr-719 January 24, 2018 14:38 Inactive

steventux reviewed Jan 24, 2018

View reviewed changes

emmabeynon force-pushed the spike-content-list-length branch from 7ef77a5 to 5753322 Compare January 24, 2018 14:59

boffbowsh temporarily deployed to government-frontend-pr-719 January 24, 2018 15:00 Inactive

steventux reviewed Jan 24, 2018

View reviewed changes

emmabeynon force-pushed the spike-content-list-length branch from 5753322 to 5bc0ef2 Compare January 24, 2018 15:16

boffbowsh temporarily deployed to government-frontend-pr-719 January 24, 2018 15:17 Inactive

emmabeynon force-pushed the spike-content-list-length branch from 5bc0ef2 to 9afbb85 Compare January 25, 2018 17:31

boffbowsh had a problem deploying to government-frontend-pr-719 February 6, 2018 11:58 Failure

steventux force-pushed the spike-content-list-length branch from b5e4cd2 to c22e4b6 Compare February 6, 2018 12:03

boffbowsh temporarily deployed to government-frontend-pr-719 February 6, 2018 12:03 Inactive

steventux force-pushed the spike-content-list-length branch from c22e4b6 to 8f38941 Compare February 6, 2018 12:11

boffbowsh temporarily deployed to government-frontend-pr-719 February 6, 2018 12:11 Inactive

emmabeynon force-pushed the spike-content-list-length branch from 8f38941 to 4253f27 Compare February 7, 2018 11:41

boffbowsh temporarily deployed to government-frontend-pr-719 February 7, 2018 11:42 Inactive

steventux force-pushed the spike-content-list-length branch from 4253f27 to 8f38941 Compare February 7, 2018 11:59

boffbowsh temporarily deployed to government-frontend-pr-719 February 7, 2018 11:59 Inactive

emmabeynon and others added 14 commits February 7, 2018 12:23

Add faker gem

07adfc3

In order to generate lorem ipsum for longer pieces of content in tests.

Show contents list depending on table row count

e44d6ce

We now show the contents list in cases where there are only two content items if the first item contains a table with more than 13 rows.

Show contents list when first item has an image and over 50 characters

8441e0a

We now show the contents list when there are 2 items on a contents lists and the first item has both an image and content over 50 characters.

Show contents list when first item has an image and over 6 rows

bdc0b5e

We now show the contents list when there are 2 items on a contents lists and the first item has both an image and a table with more than 6 rows.

Update Topical Event About Page integration tests

e1ebcf7

We update test coverage to reflect that contents lists are shown under certain conditions.

Update Specialist Documents integration tests

783cd7e

We update test coverage to reflect that contents lists are shown under certain conditions.

Update Corporate Information pages integration tests

905630e

We update test coverage to reflect that contents lists are shown under certain conditions.

Update Document Collections integration tests

71cd2a1

We update test coverage to reflect that contents lists are shown under certain conditions.

Update Statistical Data Sets integration tests

5995ae0

We update test coverage to reflect that contents lists are shown under certain conditions.

Update Detailed Guide integration tests

3c4dfa8

We update test coverage to reflect that contents lists are shown under certain conditions.

Adjust the character limit for showing content lists

f9827a7

Rename method to find_first_table for clarity

5ebd42f

emmabeynon force-pushed the spike-content-list-length branch from 8f38941 to 5ebd42f Compare February 7, 2018 13:59

boffbowsh deployed to government-frontend-pr-719 February 7, 2018 13:59 View deployment

emmabeynon merged commit a188e35 into master Feb 7, 2018

emmabeynon deleted the spike-content-list-length branch February 7, 2018 14:10

steventux mentioned this pull request Feb 9, 2018

Contents link flashes strangely #743

Closed

emmabeynon mentioned this pull request Feb 9, 2018

Fix contents list bugs #747

Merged

CodeSonia mentioned this pull request Sep 27, 2024

Handle nested divs in first_item_content #3348

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display contents lists under specific conditions #719

Display contents lists under specific conditions #719

emmabeynon commented Jan 23, 2018 •

edited

Loading

fofr Jan 23, 2018

emmabeynon Jan 23, 2018

fofr Jan 23, 2018

steventux Jan 29, 2018

fofr commented Jan 23, 2018

steventux Jan 23, 2018

steventux Jan 23, 2018

emmabeynon Jan 23, 2018

fofr Jan 25, 2018

emmabeynon commented Jan 23, 2018 •

edited

Loading

steventux Jan 23, 2018 •

edited

Loading

emmabeynon Jan 23, 2018

emmabeynon commented Jan 24, 2018

steventux Jan 24, 2018

emmabeynon Jan 24, 2018

steventux Jan 24, 2018

Display contents lists under specific conditions #719

Display contents lists under specific conditions #719

Conversation

emmabeynon commented Jan 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fofr commented Jan 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emmabeynon commented Jan 23, 2018 • edited Loading

steventux Jan 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emmabeynon commented Jan 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emmabeynon commented Jan 23, 2018 •

edited

Loading

emmabeynon commented Jan 23, 2018 •

edited

Loading

steventux Jan 23, 2018 •

edited

Loading