-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Render world locations #3337
Render world locations #3337
Conversation
f3e567f
to
5de3ee6
Compare
5de3ee6
to
896bba1
Compare
896bba1
to
25dd44d
Compare
25dd44d
to
bc622ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me (from a backend perspective). Just a small question on whether we can remove the JSON fixture file.
I'd suggest getting a frontend dev in the owning team to look at the JavaScript and CSS too.
bc622ba
to
bb8aad2
Compare
We have added the rendering of the world index page to Collections (alphagov/collections#3337), and can now switch the rendering over.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to give such a great PR description. I've made a few comments on the Ruby code.
1ec336f
to
b9e1810
Compare
Thanks for the thorough review 🙌 I've addressed most points, but left a few questions as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me! I know @hannalaakso was wanting to review the javascript
Javascript looks good to me, very nice consolidation of the search terms in the presenter meaning we can generalise the filter script. |
This adds the necessary locales to render the `/world` page.
The README contains a list of live examples. Now that the world index page is soon to be rendered by Collections, we can add it in.
We are going to make this module more generic, this renames it so that we can see the changes more easily when it's modified.
Before this JavaScript was moved from Whitehall to Collections, it was shared between the `/world` page and the `/government/organisations` page. However, since being moved it has been made specific to the `/government/organisations` page. Now that we are also moving the `/world` page to Collections, we want to make this module more generic so that it can be shared again. This also removes `data-filter-list` as it is unused.
Render a basic page for `/world` including the title and additional links.
The world index page displays a list of world locations sorted by letter. These are already sorted alphabetically in the content item, so we only need to group them.
The world index page displays a list of international delegations. These are already sorted alphabetically in the content item.
The world index page contains a JavaScript based search. We want to use the `list-filter` module for this. As the world locations on the page are grouped into letters, we need to add the concept of an `inner-block` that can be hidden when no results are present.
b9e1810
to
fd4815c
Compare
https://trello.com/c/ii6IgWnk
This is part of the work to move rendering away from Whitehall
Differences for the user compared to Whitehall
/government/organisations
page./government/organisations
page. Previously a multi-word search such as "uk p" would match all locations that contained those letters in individual words, now they must be in order./government/organisations
pageScreenshots
Commits
Add locales for world locations
This adds the necessary locales to render the
/world
page.Add world location index to the README
The README contains a list of live examples. Now that the world index page is
soon to be rendered by Collections, we can add it in.
Rename organisation list filter module
We are going to make this module more generic, this renames it so that we can
see the changes more easily when it's modified.
Make organisation list filtering JavaScript more generic
Before this JavaScript was moved from Whitehall to Collections, it was shared
between the
/world
page and the/government/organisations
page. However,since being moved it has been made specific to the
/government/organisations
page.
Now that we are also moving the
/world
page to Collections, we want to makethis module more generic so that it can be shared again.
This also removes
data-filter-list
as it is unused.Render a basic world index page
Render a basic page for
/world
including the title and additional links.Render world locations on the world index page
The world index page displays a list of world locations sorted by letter. These
are already sorted alphabetically in the content item, so we only need to group
them.
Render international delegations on the world index page
The world index page displays a list of international delegations. These
are already sorted alphabetically in the content item.
Add JavaScript filter to world index
The world index page contains a JavaScript based search. We want to use the
list-filter
module for this.As the world locations on the page are grouped into letters, we need to add the
concept of an
inner-block
that can be hidden when no results are present.Searches checked
/government/organisations
a
✅ab
✅Prime Minister's Office, 10 Downing Street
✅number 10
✅ago
✅/world
a
✅a
✅ab
✅bonaire-st-eustatius-saba
✅Bonaire/St Eustatius/Saba
✅uk p
❌Follow these steps if you are doing a Rails upgrade.