Skip to content

Commit

Permalink
Handle error state only in the first partial
Browse files Browse the repository at this point in the history 
This is so it's more consistent with the way that rails handles error on forms generally
  • Loading branch information
@mlandauer
mlandauer committed Feb 13, 2024
1 parent a290d13 commit a80cc4e
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 87 deletions.
164 changes: 83 additions & 81 deletions app/views/_tailwind/applications/_address_search.html.erb
View file
Original file line number Diff line number Diff line change
@@ -1,93 +1,95 @@
<% content_for :meta_description, Rails.configuration.planningalerts_meta_description %>

<%= render Tailwind::Heading.new(tag: :h1, extra_classes: "sr-only").with_content("Search Applications") %>
<%= render "address_search_form", q: nil, error: %>
<%= render "address_search_form", q:, error: %>

<div class="grid sm:grid-flow-col grid-rows-[repeat(4,_max-content)] auto-cols-fr gap-12 mt-20">
<%= render Tailwind::Heading.new(tag: :h2).with_content("Most commented applications") %>
<%# Wrapping in a div to make the bottom of the illustrations match up %>
<div class="flex items-end">
<%= image_tag "tailwind/illustration/people-talking.svg", alt: "" %>
</div>
<div class="space-y-8">
<% Application.trending.limit(3).each do |application| %>
<%= link_to application, class: "block focus:bg-sun-yellow" do %>
<article>
<%= render Tailwind::Heading.new(tag: :h3, color: "fuchsia", font: "sans").with_content(application.address) %>
<p class="text-2xl font-bold text-navy">Commented on <%= pluralize(application.visible_comments_count, "time") %></p>
<%# TODO: Limit length of description %>
<p class="text-2xl text-navy">for "<%= application.description %>"</p>
</article>
<% if error.nil? %>
<div class="grid sm:grid-flow-col grid-rows-[repeat(4,_max-content)] auto-cols-fr gap-12 mt-20">
<%= render Tailwind::Heading.new(tag: :h2).with_content("Most commented applications") %>
<%# Wrapping in a div to make the bottom of the illustrations match up %>
<div class="flex items-end">
<%= image_tag "tailwind/illustration/people-talking.svg", alt: "" %>
</div>
<div class="space-y-8">
<% Application.trending.limit(3).each do |application| %>
<%= link_to application, class: "block focus:bg-sun-yellow" do %>
<article>
<%= render Tailwind::Heading.new(tag: :h3, color: "fuchsia", font: "sans").with_content(application.address) %>
<p class="text-2xl font-bold text-navy">Commented on <%= pluralize(application.visible_comments_count, "time") %></p>
<%# TODO: Limit length of description %>
<p class="text-2xl text-navy">for "<%= application.description %>"</p>
</article>
<% end %>
<% end %>
<% end %>
</div>
<%= pa_link_to "View more trending applications", trending_applications_path, class: "text-2xl mt-6" %>
<%= render Tailwind::Heading.new(tag: :h2).with_content("Latest applications across Australia") %>
<div class="flex items-end">
<%= image_tag "tailwind/illustration/houses.svg", alt: "" %>
</div>
<div class="space-y-8">
<%# TODO: Probably want some ordering that shows applications from different places %>
<% Application.order(first_date_scraped: :desc).limit(3).each do |application| %>
<%= link_to application, class: "block focus:bg-sun-yellow" do %>
<article>
<%= render Tailwind::Heading.new(tag: :h3, color: "fuchsia", font: "sans").with_content(application.address) %>
<p class="text-2xl font-bold text-navy">
<%= time_ago_in_words(application.first_date_scraped) %> ago
</p>
<%# TODO: Limit length of description %>
<p class="text-2xl text-navy"><%= application.description %></p>
</article>
</div>
<%= pa_link_to "View more trending applications", trending_applications_path, class: "text-2xl mt-6" %>
<%= render Tailwind::Heading.new(tag: :h2).with_content("Latest applications across Australia") %>
<div class="flex items-end">
<%= image_tag "tailwind/illustration/houses.svg", alt: "" %>
</div>
<div class="space-y-8">
<%# TODO: Probably want some ordering that shows applications from different places %>
<% Application.order(first_date_scraped: :desc).limit(3).each do |application| %>
<%= link_to application, class: "block focus:bg-sun-yellow" do %>
<article>
<%= render Tailwind::Heading.new(tag: :h3, color: "fuchsia", font: "sans").with_content(application.address) %>
<p class="text-2xl font-bold text-navy">
<%= time_ago_in_words(application.first_date_scraped) %> ago
</p>
<%# TODO: Limit length of description %>
<p class="text-2xl text-navy"><%= application.description %></p>
</article>
<% end %>
<% end %>
<% end %>
</div>
<%= pa_link_to "View more latest applications", applications_path, class: "text-2xl mt-6" %>
</div>
<%= pa_link_to "View more latest applications", applications_path, class: "text-2xl mt-6" %>
</div>

<section class="py-12 mt-8 text-2xl border-t border-light-grey2 text-navy">
<%= render Tailwind::Heading.new(tag: :h2, extra_classes: "pb-2").with_content("Coverage") %>
<section class="py-12 mt-8 text-2xl border-t border-light-grey2 text-navy">
<%= render Tailwind::Heading.new(tag: :h2, extra_classes: "pb-2").with_content("Coverage") %>

<div class="flex flex-col-reverse items-start justify-between gap-8 pt-4 md:flex-row">
<div class="max-w-2xl">
<p>
We currently cover
<%= Authority.percentage_population_covered_by_all_active_authorities.to_i %>% of
Australia's population within <%= Authority.active.count %> authorities.
For this number we're only counting places where the main planning authority is covered.
As an example, most of Victoria is covered by the SPEAR system but not all Victorian
planning applications go through it. So, we don't include SPEAR in our population count.
</p>
<p class="pt-4">
We don't yet cover the whole country, but we are working on it.
New authorities are being added all the time.
</p>
<p class="pt-4">
If you are a programmer and would like to write a scraper for your local authority,
or work for a local authority and would like to make your data available,
<%= pa_link_to "find out how you can get involved", get_involved_path %>.
</p>
</div>
<div class="flex-shrink-0">
<%= render "authorities/coverage_percentage" %>
<div class="flex flex-col-reverse items-start justify-between gap-8 pt-4 md:flex-row">
<div class="max-w-2xl">
<p>
We currently cover
<%= Authority.percentage_population_covered_by_all_active_authorities.to_i %>% of
Australia's population within <%= Authority.active.count %> authorities.
For this number we're only counting places where the main planning authority is covered.
As an example, most of Victoria is covered by the SPEAR system but not all Victorian
planning applications go through it. So, we don't include SPEAR in our population count.
</p>
<p class="pt-4">
We don't yet cover the whole country, but we are working on it.
New authorities are being added all the time.
</p>
<p class="pt-4">
If you are a programmer and would like to write a scraper for your local authority,
or work for a local authority and would like to make your data available,
<%= pa_link_to "find out how you can get involved", get_involved_path %>.
</p>
</div>
<div class="flex-shrink-0">
<%= render "authorities/coverage_percentage" %>
</div>
</div>
</div>
</section>
</section>

<section class="py-12 text-2xl border-t text-navy border-light-grey2">
<%# TODO: Better heading? %>
<%# TODO: Do we want to include something like pb-2 for all headings in the component? %>
<%= render Tailwind::Heading.new(tag: :h2, extra_classes: "pb-2").with_content("Planning Data") %>
<div class="flex flex-col items-start justify-between gap-8 md:flex-row">
<div class="max-w-2xl">
<p class="pt-4">
From time to time planning authorities change their websites which breaks the scrapers
that gather the planning data. Help by providing a
<%= pa_link_to "donation", donate_url %> to help us fix it.
</p>
<p class="pt-4">
<%= pa_link_to "Check if planning data is broken for your authority", authorities_path %>
</p>
<section class="py-12 text-2xl border-t text-navy border-light-grey2">
<%# TODO: Better heading? %>
<%# TODO: Do we want to include something like pb-2 for all headings in the component? %>
<%= render Tailwind::Heading.new(tag: :h2, extra_classes: "pb-2").with_content("Planning Data") %>
<div class="flex flex-col items-start justify-between gap-8 md:flex-row">
<div class="max-w-2xl">
<p class="pt-4">
From time to time planning authorities change their websites which breaks the scrapers
that gather the planning data. Help by providing a
<%= pa_link_to "donation", donate_url %> to help us fix it.
</p>
<p class="pt-4">
<%= pa_link_to "Check if planning data is broken for your authority", authorities_path %>
</p>
</div>
<%= image_tag "tailwind/illustration/mechanic.svg", alt: "" %>
</div>
<%= image_tag "tailwind/illustration/mechanic.svg", alt: "" %>
</div>
</section>
</section>
<% end %>
5 changes: 2 additions & 3 deletions app/views/_tailwind/applications/_address_with_results.html.erb
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<% content_for :meta_description, Rails.configuration.planningalerts_meta_description %>

<%= render Tailwind::Heading.new(tag: :h1, extra_classes: "sr-only").with_content("Search Applications") %>
<%= render "address_search_form", q:, error: %>
<%= render "address_search_form", q:, error: nil %>

<%# TODO: Need a proper design for this section %>
<%# TODO: When this section shows up it's not actually clear what the current search is for %>
Expand All @@ -19,11 +19,10 @@
</section>
<% end %>

<%# TODO: Do we want to keep this template for when there are no errors? %>
<%# TODO: Remove duplication of heading %>
<%# TODO: Figure out how to handle no results when extra_options_on_address_search is enabled %>
<%# TODO: Add filter option for no filtering (e.g. any distance or any time) because the filtering in its current is very confusing in its behaviour %>
<% if error.nil? && applications.empty? && !Flipper.enabled?(:extra_options_on_address_search, current_user)%>
<% if applications.empty? && !Flipper.enabled?(:extra_options_on_address_search, current_user)%>
<section class="pt-16">
<%= render "no_applications", full_address:, q: %>
</section>
Expand Down
6 changes: 3 additions & 3 deletions app/views/_tailwind/applications/address.html.erb
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<%# TODO: It would be more sensible to render different views from the controller (or use routing) rather than doing it here %>
<% if @q.nil? %>
<%= render "address_search", error: @error %>
<% if @q.nil? || @error %>
<%= render "address_search", q: @q, error: @error %>
<% else %>
<%= render "address_with_results", q: @q, error: @error, applications: @applications, sort: @sort, page: @page, alert: @alert, other_addresses: @other_addresses, full_address: @full_address, radius: @radius, time: @time %>
<%= render "address_with_results", q: @q, applications: @applications, sort: @sort, page: @page, alert: @alert, other_addresses: @other_addresses, full_address: @full_address, radius: @radius, time: @time %>
<% end %>

0 comments on commit a80cc4e

Please sign in to comment.