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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix menu home highlight when using different locales #2095

Merged
merged 1 commit into from
Oct 23, 2017

Conversation

mrcasals
Copy link
Contributor

馃帺 What? Why?

Fixes the menu Home element highlight when the user is logged in.

馃搶 Related Issues

None

馃摲 Screenshots (optional)

Before this PR:

http://recordit.co/3KFDe2Vwz4
Description

After this PR:

http://recordit.co/VaaInJckOy

@mrcasals mrcasals self-assigned this Oct 23, 2017
oriolgual
oriolgual previously approved these changes Oct 23, 2017
@deivid-rodriguez
Copy link
Contributor

deivid-rodriguez commented Oct 23, 2017

I think we have infrastructure to get this easily tested. See

# frozen_string_literal: true
require "spec_helper"
describe "Menu", type: :feature do
let(:organization) { create(:organization) }
before do
switch_to_host(organization.host)
visit decidim.root_path
end
matcher :have_selected_option do |expected|
match do |page|
page.has_selector?(".main-nav__link--active", count: 1) &&
page.has_selector?(".main-nav__link--active", text: expected)
end
end
it "renders the default main menu" do
within ".main-nav" do
expect(page).to \
have_selector("li", count: 2) &
have_link("Home", href: "/") &
have_link("More information", href: "/pages")
end
end
it "selects the correct default active option" do
within ".main-nav" do
expect(page).to have_selected_option("Home")
end
end
context "when clicking on a menu entry" do
before do
click_link "More information"
end
it "switches the active option" do
expect(page).to have_selected_option("More information")
end
context "and clicking on a subpage of that entry" do
before do
page = create(:static_page, organization: organization)
visit current_path
click_link page.title["en"]
end
it "preserves the active option" do
expect(page).to have_selected_option("More information")
end
end
end
end

@codecov
Copy link

codecov bot commented Oct 23, 2017

Codecov Report

Merging #2095 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2095      +/-   ##
==========================================
- Coverage   98.55%   98.54%   -0.02%     
==========================================
  Files        1174     1174              
  Lines       26836    26847      +11     
==========================================
+ Hits        26449    26456       +7     
- Misses        387      391       +4

@mrcasals
Copy link
Contributor Author

@deivid-rodriguez I've added a test for this, test fails without the change in engine.rb 馃槃

Copy link
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Yuppi!

@mrcasals mrcasals merged commit 14dc595 into master Oct 23, 2017
@mrcasals mrcasals deleted the fix/menu-home-highlight branch October 23, 2017 13:05
@ghost ghost removed the in-review label Oct 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants