Skip to content

Commit cb1bb67

Browse files
Feature/755 make links and dropdowns turbo 8 friendly (#809)
* Use links in people dropdown and make them work with prefetch * Make language selection dropdown work again * Increase version number * Change minor instead of patch version in new version number * Make links in people search dropdown take in full height and width * Make slim select only follow link if value is link * Manually call before change event callback in slim select testhelpers * Make language dropdown a slim select dropdown to be able to prefetch locales * Make rubocop happy * Bring back old language selection * Make link prefetching work in cv search * Refactor cv_search_spec * Fix flaky cv search specs * Use document.location.href instead of turbo.visit for language dropdown * Remove obsolete data-html attribute from i18n dropdown data * Make prefetching of people in skill search results work * Disable link of selected element in slim select dropdown to allow opening of dropdown * Disable link prefetching on skills csv export link * Redisable link prefetching in dev * Remove unnecessary check in dropdown controller and rename handleChange to navigateOnChange * Use windows.location instead of document.location * Remove duplicate logic
1 parent c194571 commit cb1bb67

File tree

11 files changed

+65
-17
lines changed

11 files changed

+65
-17
lines changed

app/assets/stylesheets/styles.scss

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,18 @@ pzsh-topbar {
310310
opacity: 1;
311311
}
312312

313+
.dropdown-option-link {
314+
width: 100%;
315+
display: block;
316+
text-decoration: none;
317+
color: black;
318+
padding: 5px 7px;
319+
}
320+
321+
.ss-main .dropdown-option-link {
322+
padding: 0;
323+
}
324+
313325
.dropdown-option-highlighted {
314326
background-color: #2c97a6;
315327
}

app/helpers/person_helper.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,14 @@ def sorted_people
8282
end
8383

8484
def people_for_select
85-
Person.all.map { |p| [p.name, person_path(p)] }
85+
Person.all.map do |p|
86+
[
87+
p.name, person_path(p),
88+
{
89+
'data-html': "<a href='#{person_path(p)}' class='dropdown-option-link'>#{p.name}</a>",
90+
class: 'p-0'
91+
}
92+
]
93+
end
8694
end
8795
end

app/javascript/controllers/dropdown_controller.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,29 @@ export default class extends Controller {
88
if (!this.hasDropdownTarget)
99
return;
1010

11-
new SlimSelect({
12-
select: this.dropdownTarget
11+
const slimSelectDropdown = new SlimSelect({
12+
select: this.dropdownTarget,
13+
events: {
14+
beforeChange: (newVal) => {
15+
newVal = newVal[0];
16+
17+
// Check if dropdown element is a link
18+
if(newVal.html.startsWith("<a")) {
19+
Turbo.visit(newVal.value);
20+
21+
return false;
22+
}
23+
}
24+
},
1325
});
26+
27+
// Make currently selected element not follow link when clicked on, so opening the dropdown is possible
28+
if(slimSelectDropdown.getSelected()[0]?.startsWith("/")) {
29+
document.querySelector('.ss-main .dropdown-option-link').href = "javascript:void(0)";
30+
}
1431
}
1532

16-
handleChange(event) {
33+
navigateOnChange(event) {
1734
window.location.href = event.target.value;
1835
}
1936
}

app/javascript/controllers/highlight_controller.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Controller } from "@hotwired/stimulus"
22

33
export default class extends Controller {
44
connect() {
5-
const params = new URLSearchParams(window.location.search);
5+
const params = new URL(window.location.href).searchParams;
66
if(params.has("q")) {
77
window.find(params.get("q"))
88
}

app/views/cv_search/index.html.haml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@
1212
- else
1313
- @cv_search_results.each do |result|
1414
%div.w-50.d-flex
15-
= link_to result[:person][:name], person_path(result[:person][:id]), {class: "bg-skills-green w-50 text-decoration-none text-white ps-1 p-2 rounded-1", "data-turbo": "false"}
15+
= link_to result[:person][:name], person_path(result[:person][:id]), {class: "bg-skills-green w-50 text-decoration-none text-white ps-1 p-2 rounded-1", "data-turbo-frame": "_top" }
1616
%div.w-50.d-flex.justify-content-end.align-items-center
1717
%div.me-1
1818
= ti("search.found_in")
1919
- url_params = {q: params[:q], rating: found_in_skills?(result)? 1: nil}
2020
- person_id = result[:person][:id]
2121
- path = person_path(person_id, url_params)
2222
- path = person_people_skills_path(person_id, url_params) if found_in_skills?(result)
23-
= link_to result[:found_in], path, {class: "bg-skills-search-result-blue w-50 text-decoration-none text-white ps-1 p-2 rounded-1 text-center", "data-turbo": "false"}
23+
= link_to result[:found_in], path, {class: "bg-skills-search-result-blue w-50 text-decoration-none text-white ps-1 p-2 rounded-1 text-center", "data-turbo-frame": "_top" }
2424
%br

app/views/layouts/application.html.haml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@
2121
%div.d-flex
2222
%div.mb-1.mt-1
2323
%img{:src=> "/assets/logo.svg",:height=>"32"}
24-
%text.d-flex.align-items-end.ms-2.small= "5.0.0"
24+
%text.d-flex.align-items-end.ms-2.small= "5.1.0"
2525
%ul.navbar.text-gray
2626
-# Language selector
2727
%li.d-flex.align-items-center.cursor-pointer.border-start.border-end.h-100.ps-2.pe-2{"data-controller": "dropdown"}
28-
= select :i18n, :language, language_selector, {}, class: "form-control", data:{action: "change->dropdown#handleChange"}
28+
= select :i18n, :language, language_selector, {}, class: "form-control", data:{action: "change->dropdown#navigateOnChange"}
2929
-# Devise/Mockdata
3030
- if Rails.env.development?
3131
%li.d-flex.align-items-center.cursor-pointer.ps-2.pe-2.border-start.border-end.h-100

app/views/people/_search.html.haml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
%div.d-flex.col-9.gap-3
33
%span.col-6{"data-controller": "dropdown"}
44
- selected_path = model_path_or_nil(person)
5-
= select :person_id, :person, options_for_select(sorted_people, select_when_available(selected_path)), {prompt: person.nil?}, {data:{"dropdown-target": "dropdown" , action: "change->dropdown#handleChange"}}
5+
= select :person_id, :person, options_for_select(sorted_people, select_when_available(selected_path)), {prompt: person.nil?}, {data:{"dropdown-target": "dropdown"}}
66
%div.d-flex.align-items-center.text-gray
77
= "#{t '.updated_at'}: #{@person&.last_updated_at.strftime("%d.%m.%Y")}" if @person&.last_updated_at
88
%a.d-flex.justify-content-between#new-person-button

app/views/people_skills/index.html.haml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
- entries.each do | filtered_entry |
2222
%div.d-flex.flex-row.w-100.border-bottom
2323
%div.d-flex.flex-column.mt-2.mb-2.person-filter-container.justify-content-start
24-
= link_to filtered_entry[:person].name, person_path(filtered_entry[:person]), {"data-turbo"=>false}
24+
= link_to filtered_entry[:person].name, person_path(filtered_entry[:person]), {"data-turbo-frame": "_top"}
2525
%div.d-flex.flex-column.mt-2.mb-2.skills-filter-container
2626
- filtered_entry[:skills].each do |person_skill|
2727
%div.d-flex.flex-row.mb-5

app/views/skills/_header.html.haml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@
2222
%span.text-gray.me-1
2323
= "#{Category.model_name.human}:"
2424
= form.collection_select :category, Category.all_parents.order(:title), :id, :title, {prompt: true}, class: "form-select fit-content", onchange: 'this.form.requestSubmit();'
25-
= export_action_link export_skills_path
25+
= export_action_link export_skills_path, "data-turbo-prefetch": false
2626
= add_action_link_modal

spec/features/cv_search_spec.rb

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,19 @@
3535
it 'should open person when clicking result' do
3636
fill_in 'cv_search_field', with: person.projects.first.technology
3737
check_search_results(Person.human_attribute_name(:projects))
38-
first("a", text: person.name).click();
38+
# Tests are flaky in firefox
39+
sleep 0.3
40+
click_link person.name
3941
expect(page).to have_current_path(person_path(person))
4042

4143
visit(cv_search_index_path)
4244
education_location = person.educations.first.location
4345
fill_in 'cv_search_field', with: education_location
4446
check_search_results(Person.human_attribute_name(:educations))
47+
# Tests are flaky in firefox
48+
sleep 0.3
4549
click_link(Person.human_attribute_name(:educations))
46-
expect(current_url).to end_with(person_path(person, q: education_location))
50+
expect(page).to have_current_path(person_path(person, q: education_location))
4751
end
4852

4953
it 'should only display results when length of search-text is > 3' do
@@ -68,7 +72,7 @@
6872

6973
def check_search_results(field_name)
7074
within('turbo-frame#search-results') {
71-
expect(page).to have_content(person.name)
72-
expect(page).to have_content(field_name)
75+
expect(page).to have_link(person.name)
76+
expect(page).to have_link(field_name)
7377
}
7478
end

spec/support/slimselect_helpers.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
module SlimselectHelpers
22
def select_from_slim_select(selector, option_text, create_if_missing=false)
33
ss_open(selector)
4-
if(create_if_missing)
4+
if create_if_missing
55
ss_create(selector, option_text)
66
else
77
ss_select_text(selector, option_text)
@@ -51,6 +51,13 @@ def ss_select_text(selector, option_text)
5151

5252
def ss_select(selector, option_value)
5353
call(selector, "setSelected", option_value)
54+
55+
# Manually call the beforeChange event because we manually update the slim select object, which doesn't trigger the event callback
56+
57+
# Check if option_value is a path
58+
if option_value.starts_with? '/'
59+
evaluate_script("document.querySelector('#person_id_person').slim.events.beforeChange([{value: '#{option_value}', html: '<a></a>'}])")
60+
end
5461
end
5562

5663
def ss_search(selector, text)

0 commit comments

Comments
 (0)