Skip to content

Fix Anchor Link triggers transition #361

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

Merged
merged 4 commits into from
Feb 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ group :development, :test do
gem 'puma'
gem 'simplecov', require: false, group: :test
gem 'byebug'
# gem 'pry-byebug'
gem 'pry-byebug'
gem 'webmock'
gem 'webdrivers', '~> 4.1'
end
Expand Down
8 changes: 8 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ GEM
cells (>= 4.1.6, < 5.0.0)
childprocess (0.9.0)
ffi (~> 1.0, >= 1.0.11)
coderay (1.1.2)
concurrent-ruby (1.1.5)
crack (0.4.3)
safe_yaml (~> 1.0.0)
Expand Down Expand Up @@ -125,6 +126,12 @@ GEM
nokogiri (1.10.5)
mini_portile2 (~> 2.4.0)
pipetree (0.1.1)
pry (0.12.2)
coderay (~> 1.1.0)
method_source (~> 0.9.0)
pry-byebug (3.8.0)
byebug (~> 11.0)
pry (~> 0.10)
public_suffix (3.0.3)
puma (4.3.1)
nio4r (~> 2.0)
Expand Down Expand Up @@ -253,6 +260,7 @@ DEPENDENCIES
cells-rails
generator_spec
matestack-ui-core!
pry-byebug
puma
rspec-rails (~> 3.8)
selenium-webdriver
Expand Down
9 changes: 5 additions & 4 deletions app/concepts/matestack/ui/core/app/app.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Vue from 'vue/dist/vue.esm'
import axios from 'axios'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

wasn't used anywhere, in the file so I removed it. I hope I'm not unaware of some side effect this had, but the tests pass 🤷‍♂️

import VRuntimeTemplate from "v-runtime-template"
import Vuex from 'vuex'
import isNavigatingToAnotherPage from "app/location"

const componentDef = {
props: ['appConfig', 'params'],
Expand All @@ -12,9 +12,10 @@ const componentDef = {
asyncTemplate: state => state.pageTemplate,
}),
mounted: function(){
const self = this;
window.onpopstate = function(event) {
self.$store.dispatch("navigateTo", {url: document.location.pathname, backwards: true} );
window.onpopstate = (event) => {
if (isNavigatingToAnotherPage(document.location, event)) {
this.$store.dispatch("navigateTo", {url: document.location.pathname, backwards: true} );
};
}
},
components: {
Expand Down
10 changes: 10 additions & 0 deletions app/concepts/matestack/ui/core/app/location.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const isNavigatingToAnotherPage = function(currentLocation, popstateEvent) {
const targetLocation = popstateEvent.target.location;

// omits hash by design
return currentLocation.pathname !== targetLocation.pathname ||
currentLocation.origin !== targetLocation.origin ||
currentLocation.search !== targetLocation.search
}

export default isNavigatingToAnotherPage
2 changes: 2 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@

Dir[File.join File.dirname(__FILE__), 'support', '**', '*.rb'].each { |f| require f }

require 'pry'

RSpec.configure do |config|
# config.include Capybara::DSL
# rspec-expectations config goes here. You can use an alternate
Expand Down
136 changes: 136 additions & 0 deletions spec/usage/components/link_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -179,4 +179,140 @@ def response
expect(stripped(static_output)).to ( include(stripped(expected_static_output)) )
end

it "behaves correctly with anchor links (no reload, retain anchor)" do
class ExamplePage < Matestack::Ui::Page
def response
components {
link path: "#someanchor", text: "go to anchor", id: "my-link"

br times: 200

div id: "someanchor" do
plain "hello!"
end

div id: "my-div" do
plain "#{DateTime.now.strftime('%Q')}"
end
}
end
end

visit "/example"

element = page.find("#my-div")
before_content = element.text

# don't you rerender on me!
expect(ExamplePage).not_to receive(:new)

page.click_link("my-link")

# if the page reloaded we'd have different content here but as we don't want reloads
# we want the same
expect(page).to have_css("#my-div", text: before_content)
expect(page.current_url).to end_with("#someanchor")
end

describe "with an App" do

after :each do
class ExamplePage < Matestack::Ui::Page
# as the test suites work with redefining classes and this
# hack here was used to set a specific app easily,
# we need to "restore" previous state
def set_app_class
super
end
end
end

it "behaves correctly with anchor links (no reload, retain anchor) even inside an app" do
class Apps::MyTestApp < Matestack::Ui::App
def response
components {
page_content
}
end
end

class ExamplePage < Matestack::Ui::Page
def response
components {
link path: "#someanchor", text: "go to anchor", id: "my-link"

br times: 200

div id: "someanchor" do
plain "hello!"
end

div id: "my-div" do
plain "#{DateTime.now.strftime('%Q')}"
end
}
end

# Hacky/instable but easy way to set my custom App for this page
def set_app_class
@app_class = Apps::MyTestApp
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

as the comment states, this is hacky but considering doing naming convetions, a new controller and appending to the routes etc. this seemed preferable to me.

end
end

visit "/example"

element = page.find("#my-div")
before_content = element.text

# don't you rerender on me!
expect(ExamplePage).not_to receive(:new)

page.click_link("my-link")

# if the page reloaded we'd have different content here but as we don't want reloads
# we want the same
expect(page).to have_css("#my-div", text: before_content)
expect(page.current_url).to end_with("#someanchor")
end

it "just changing the search string will still reload the page" do
class Apps::MyTestApp < Matestack::Ui::App
def response
components {
page_content
}
end
end

class ExamplePage < Matestack::Ui::Page
def response
components {
link path: "?a=true", text: "go to anchor", id: "my-link"

br times: 200

div id: "my-div" do
plain "#{DateTime.now.strftime('%Q')}"
end
}
end

# Hacky/instable but easy way to set my custom App for this page
def set_app_class
@app_class = Apps::MyTestApp
end
end

visit "/example"

element = page.find("#my-div")
before_content = element.text

page.click_link("my-link")

expect(page).to have_css("#my-div")
expect(page).to have_no_css("#my-div", text: before_content)
expect(page.current_url).to end_with("?a=true")
end
end
end
Loading