Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Placements index: Move location search to filter sidebar #1096

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Placements index: Move location search to filter sidebar #1096

wants to merge 2 commits into from

Conversation

dp-daly
Copy link
Contributor

@dp-daly dp-daly commented Oct 8, 2024

Context

These design changes are in preparation of a new travel time feature on the placements index page.

Changes proposed in this pull request

  • Page title and H1 change: “Find placements”.
  • Location search bar moved to filter sidebar.
  • Location search included as a selected filter tag.
  • Location search cleared by clearing filters.
  • New paragraph tag shows "results sorted by" either alphabetical order or distance from the search location.
  • Number of results move to results column.
  • The structure of the titles of the search results has changed to [Subject] en dash [School name]. This means the links remain understandable for accessibility reasons, but puts the subject first.
  • The size of the link becomes govuk-heading-m to make it stand out from the content and give visual hierarchy.

Guidance to review

  • Log in as a provider user.
  • You will land on the placements index page.
  • Search a location for which there will be a result (e.g. a town or city close to seeded placements).
  • Check that the location appears as a selected filter tag.
  • Check that results are rendered as expected (is the results number correct?; does the paragraph say "results sorted by distance from [the location you searched]"?)
  • Check that the location clears when the filter tag is cancelled or the 'clear filters' button is pressed.
  • Search a location for which there will be no result.
  • Check that a 'no results' message appears and the 'sorted by' paragraph is not visible.
  • Use a combination of filters to make sure both the filter and search interact as you would expect.

Link to Trello card

https://trello.com/c/oYsYxDX2/854-layout-changes-to-placements-search-page

Screenshots

Screen.Recording.2024-10-08.at.17.24.15.mov

@dp-daly dp-daly requested review from a team as code owners October 8, 2024 16:25
@dp-daly dp-daly self-assigned this Oct 8, 2024
@dp-daly
Location search moved to filter sidebar; filter tag included for loca…
68ff4bb 
…tion searches; search results component format changes.
@dp-daly dp-daly force-pushed the dpd/update-placements-index-design branch from c2f20a3 to cf97525 Compare October 9, 2024 12:46
@dp-daly dp-daly force-pushed the dpd/update-placements-index-design branch from cf97525 to 1059c23 Compare October 9, 2024 15:00
@dp-daly dp-daly added the deploy A Review App will be created for PRs with this label label Oct 9, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 9, 2024

Deployments

App URL
Track & Pay https://track-and-pay-1096.test.teacherservices.cloud
School Placements https://manage-school-placements-1096.test.teacherservices.cloud

JamieCleare2525
JamieCleare2525 reviewed Oct 10, 2024
View reviewed changes
app/forms/placements/placements/filter_form.rb
@@ -20,10 +21,10 @@ def filters_selected?
attributes.except("placements_to_show", "academic_year_id").values.compact.flatten.any?
end

def clear_filters_path(search_location: nil)
def clear_filters_path(*)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need the argument here anymore

JamieCleare2525
JamieCleare2525 reviewed Oct 10, 2024
View reviewed changes
Copy link
Contributor

@JamieCleare2525 JamieCleare2525 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are a few places which still pass search_location as an argument, which no longer need to because search_location is now a filter.

app/forms/placements/placements/filter_form.rb
@@ -36,7 +37,8 @@ def index_path_without_filter(filter:, value: nil, search_location: nil)

placements_provider_placements_path(
@provider,
params: { filters: without_filter, search_location: },
search_location,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the search_location included in the without_filter? If yes, you can probably remove search_location

app/views/placements/providers/placements/_filter.html.erb
filter_form.index_path_without_filter(
filter: "search_location",
value: @search_location,
search_location:,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given search_location is now a filter, you no longer need to pass search_location into any of these index_path_without_filter methods on this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JamieCleare2525 JamieCleare2525 JamieCleare2525 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@dp-daly dp-daly

Labels
deploy A Review App will be created for PRs with this label
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dp-daly @JamieCleare2525