maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[deploy] Remove sort links from tags for logged-out (SEO) (#8932)

Browse source 
* Remove sort links from tags for logged-out (SEO)

* Fix tests

* Fix tests take 2

* Final test fix

* Read error message more clearly

* Adjust tests

* Final test fix
This commit is contained in:
Ben Halpern 2020-06-26 17:21:18 -04:00 • committed by GitHub
parent 0f6801ea99
commit 8e0972dd33
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 70 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

80
app/views/articles/tag_index.html.erb
View file

@ -56,53 +56,43 @@
}
</style>
<% end %>
<div class="on-page-nav-controls" id="on-page-nav-controls">
<div class="on-page-nav-label sub-home-nav">
<div class="wide-nav-links">
<a class="nav-chronofiter-link <%= "selected" if %w[week month year infinity latest].exclude?(params[:timeframe]) %>" href="<%= list_path %>">
FEED
</a>
<span class="separator"></span>
<a class="nav-chronofiter-link <%= "selected" if timeframe_check("week") %>" href="<%= list_path %>/top/week">
WEEK
</a>
<a class="nav-chronofiter-link <%= "selected" if timeframe_check("month") %>" href="<%= list_path %>/top/month">
MONTH
</a>
<a class="nav-chronofiter-link <%= "selected" if timeframe_check("year") %>" href="<%= list_path %>/top/year">
YEAR
</a>
<a class="nav-chronofiter-link <%= "selected" if timeframe_check("infinity") %>" href="<%= list_path %>/top/infinity">
INFINITY
</a>
<span class="separator"></span>
<a class="nav-chronofiter-link <%= "selected" if timeframe_check("latest") %>" href="<%= list_path %>/latest">
LATEST
</a>
</div>
<div class="narrow-nav-select" aria-label="feed-filter-select">
<% if ["week", "month", "year", "infinity", "latest"].exclude?(params[:timeframe]) %>
<button type="button" id="narrow-feed-butt">&lt;MY <%= community_name %> FEED&gt;</button>
<% elsif timeframe_check('week') %>
<button type="button" id="narrow-feed-butt">&lt;PAST WEEK&gt;</button>
<% elsif timeframe_check('month') %>
<button type="button" id="narrow-feed-butt">&lt;PAST MONTH&gt;</button>
<% elsif timeframe_check('year') %>
<button type="button" id="narrow-feed-butt">&lt;PAST YEAR&gt;</button>
<% elsif timeframe_check('infinity') %>
<button type="button" id="narrow-feed-butt">&lt;INFINITY&gt;</button>
<% elsif timeframe_check('latest') %>
<button type="button" id="narrow-feed-butt">&lt;LATEST&gt;</button>
<% end %>
</div>
</div>
<button class="on-page-nav-butt on-page-nav-butt-left" id="on-page-nav-butt-left" aria-label="nav-button-left">
<img src="<%= asset_path "stack.svg" %>" alt="left-sidebar-nav">
<header class="flex items-center p-2 m:p-0 m:pb-2" id="on-page-nav-controls">
<button type="button" class="crayons-btn crayons-btn--ghost crayons-btn--icon mr-2 inline-block m:hidden" id="on-page-nav-butt-left" aria-label="nav-button-left">
<%= inline_svg_tag("stack.svg", aria: true, width: 24, height: 24, class: "crayons-icon", title: "Left sidebar") %>
</button>
<button class="on-page-nav-butt on-page-nav-butt-right" id="on-page-nav-butt-right" aria-label="nav-button-right">
<img src="<%= asset_path "lightning.svg" %>" alt="right-sidebar-nav">
<% if user_signed_in? %>
<div class="flex-1 flex items-center justify-between">
<h2 class="fs-l fw-heavy">Posts</h2>
<nav class="crayons-tabs hidden s:flex">
<a href="<%= list_path %>" class="crayons-tabs__item <%= "crayons-tabs__item--current" if %w[week month year infinity latest].exclude?(params[:timeframe]) %>">Feed</a>
<a href="<%= list_path %>/top/week" class="crayons-tabs__item <%= "crayons-tabs__item--current" if timeframe_check("week") %>">Week</a>
<a href="<%= list_path %>/top/month" class="crayons-tabs__item <%= "crayons-tabs__item--current" if timeframe_check("month") %>">Month</a>
<a href="<%= list_path %>/top/year" class="crayons-tabs__item <%= "crayons-tabs__item--current" if timeframe_check("year") %>">Year</a>
<a href="<%= list_path %>/top/infinity" class="crayons-tabs__item <%= "crayons-tabs__item--current" if timeframe_check("infinity") %>">Infinity</a>
<a href="<%= list_path %>/latest" class="crayons-tabs__item <%= "crayons-tabs__item--current" if timeframe_check("latest") %>">Latest</a>
</nav>
<select class="crayons-select s:hidden ml-2 s:ml-auto w-auto" id="feed-filter-select" aria-label="feed-filter-select">
<option value="<%= list_path %>" <% if ["week", "month", "year", "infinity", "latest"].exclude?(params[:timeframe]) %> selected<% end %>>Feed</option>
<option value="<%= list_path %>/top/week" <% if timeframe_check("week") %> selected<% end %>>Week</option>
<option value="<%= list_path %>/top/month" <% if timeframe_check("month") %> selected<% end %>>Month</option>
<option value="<%= list_path %>/top/year" <% if timeframe_check("year") %> selected<% end %>>Year</option>
<option value="<%= list_path %>/top/infinity" <% if timeframe_check("infinity") %> selected<% end %>>Infinity</option>
<option value="<%= list_path %>/latest" <% if timeframe_check("latest") %> selected<% end %>>Latest</option>
</select>
</div>
<% else %>
<div class="flex-1 flex items-center justify-between">
<div class="crayons-notice w-100">👋 <a href="/enter">Sign in</a> for the ability sort posts by <strong>top</strong> and <strong>latest</strong>.</div>
</div>
<% end %>
<button type="button" class="crayons-btn crayons-btn--ghost crayons-btn--icon ml-2 inline-block l:hidden" id="on-page-nav-butt-right" aria-label="nav-button-right">
<%= inline_svg_tag("lightning.svg", aria: true, width: 24, height: 24, class: "crayons-icon", title: "Right sidebar") %>
</button>
</div>
</header>
<div class="substories" id="substories">
<%# articles/tags/main_feed will iterate on stories with .each_with_index,

2
app/views/articles/tags/_meta.html.erb
View file

@ -1,5 +1,5 @@
<% title_with_timeframe(
page_title: @tag.capitalize,
page_title: @tag.capitalize + (@page.present? && @page > 1 ? " Page #{@page}" : "" ),
timeframe: params[:timeframe],
content_for: true,
) %>

23
spec/requests/stories_index_spec.rb
View file

@ -331,6 +331,11 @@ RSpec.describe "StoriesIndex", type: :request do
sign_in user
end
it "shows tags to signed-in users" do
get "/t/#{tag.name}"
expect(response.body).to include("crayons-tabs__item crayons-tabs__item--current")
end
it "has mod-action-button" do
get "/t/#{tag.name}"
expect(response.body).to include('<a class="cta mod-action-button"')
@ -351,6 +356,12 @@ RSpec.describe "StoriesIndex", type: :request do
context "without user signed in" do
let(:tag) { create(:tag) }
it "shows sign-in notice to non-signed-in users" do
get "/t/#{tag.name}"
expect(response.body).not_to include("crayons-tabs__item crayons-tabs__item--current")
expect(response.body).to include("for the ability sort posts by")
end
it "does not render pagination" do
get "/t/#{tag.name}"
expect(response.body).not_to include('<span class="olderposts-pagenumber">')
@ -368,6 +379,18 @@ RSpec.describe "StoriesIndex", type: :request do
expect(response.body).not_to include('<div id="sidebar-wrapper-right"')
end
it "renders proper page title for page 1" do
create_list(:article, 20, user: user, featured: true, tags: [tag.name], score: 20)
get "/t/#{tag.name}/page/1"
expect(response.body).to include("<title>#{tag.name.capitalize} - ")
end
it "renders proper page title for page 2" do
create_list(:article, 20, user: user, featured: true, tags: [tag.name], score: 20)
get "/t/#{tag.name}/page/2"
expect(response.body).to include("<title>#{tag.name.capitalize} Page 2 - ")
end
it "does not include current page link" do
create_list(:article, 20, user: user, featured: true, tags: [tag.name], score: 20)
get "/t/#{tag.name}/page/2"

30
spec/system/articles/user_visits_articles_by_tag_spec.rb
View file

@ -12,7 +12,9 @@ RSpec.describe "User visits articles by tag", type: :system do
context "when user hasn't logged in" do
context "when 2 articles" do
before { visit "/t/javascript" }
before do
visit "/t/javascript"
end
it "shows the header", js: true, stub_elasticsearch: true do
within("h1") { expect(page).to have_text("javascript") }
@ -22,14 +24,6 @@ RSpec.describe "User visits articles by tag", type: :system do
within("h1") { expect(page).to have_button("Follow") }
end
it "shows time buttons" do
within("#on-page-nav-controls") do
expect(page).to have_link("WEEK", href: "/t/javascript/top/week")
expect(page).to have_link("INFINITY", href: "/t/javascript/top/infinity")
expect(page).to have_link("LATEST", href: "/t/javascript/latest")
end
end
it "shows correct articles count" do
expect(page).to have_selector(".crayons-story", count: 2)
end
@ -41,16 +35,6 @@ RSpec.describe "User visits articles by tag", type: :system do
expect(page).not_to have_text(article2.title)
end
end
it "when user clicks 'week'", js: true, stub_elasticsearch: true do
click_on "WEEK"
within("#articles-list") do
expect(page).to have_text(article.title)
expect(page).not_to have_text(article3.title)
expect(page).not_to have_text(article2.title)
end
end
end
context "when more articles" do
@ -75,5 +59,13 @@ RSpec.describe "User visits articles by tag", type: :system do
within("h1") { expect(page).to have_button("Following") }
end
it "shows time buttons" do
within("#on-page-nav-controls") do
expect(page).to have_link("Week", href: "/t/functional/top/week")
expect(page).to have_link("Infinity", href: "/t/functional/top/infinity")
expect(page).to have_link("Latest", href: "/t/functional/latest")
end
end
end
end