Skip to content

Commit

Permalink
Merge pull request #16999 from opf/bug/53767-user-is-not-taken-to-the…
Browse files Browse the repository at this point in the history
…-secondary-input-field-after-selecting-a-filter

[#53767] fix: focus input field after selecting a filter
  • Loading branch information
oliverguenther authored Oct 21, 2024
2 parents ddb9bd5 + 9bca254 commit 47ed900
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 14 deletions.
1 change: 1 addition & 0 deletions app/views/filters/_autocomplete.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# Stimulus controller.
}.merge(autocomplete_options.except(:component)),
class: 'form--field',
id: "#{filter.name}_value",
data: {
'filter-autocomplete': true,
'filter--filters-form-target': 'filterValueContainer',
Expand Down
1 change: 1 addition & 0 deletions app/views/filters/_boolean.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
name: "v-" + filter.class.key.to_s,
checked: filter.values.first != 'f',
filterName: 'boolean',
id: "#{filter.name}_value"
}
%>
</label>
Expand Down
3 changes: 2 additions & 1 deletion app/views/filters/date/_days.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
<span class="inline-label">
<%= number_field_tag :value,
value,
id: "#{filter.name}_value",
class: 'advanced-filters--text-field -slim',
'data-filter--filters-form-target': 'days',
'data-filter-name': filter.name %>
<label for="value" class="form-label -transparent">days</label>
<label for="<%= "#{filter.name}_value" %>" class="form-label -transparent">days</label>
</span>
</div>
</div>
Expand Down
3 changes: 2 additions & 1 deletion app/views/filters/list/_select.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
{
class: 'form--select -slim',
'data-filter--filters-form-target': 'filterValueSelect',
'data-filter-name': filter.name
'data-filter-name': filter.name,
id: "#{filter.name}_value"
}]
if multi_value
select_options.third[:multiple] = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,39 @@ export default class FiltersFormController extends Controller {
this.addFilterSelectTarget.selectedIndex = 0;
this.setSpacerVisibility();

this.focusFilterValueIfPossible(selectedFilter);

if (this.performTurboRequestsValue) {
this.sendForm();
}
}

// Takes an Element and tries to find the next input or select child element. This should be the filter value.
// If found, it will be focused.
focusFilterValueIfPossible(element:undefined|HTMLElement) {
if (!element) return;

// Try different selectors for various filter styles. The order is important as some selectors match unwanted
// hidden fields when used too early in the chain.
const selectors = [
'.advanced-filters--filter-value ng-select input',
'.advanced-filters--filter-value input',
'.advanced-filters--filter-value select',
];

selectors.some((selector) => {
const target = element.querySelector(selector) as HTMLElement;

if (target) {
target.focus();
// We have found and focused our element, abort the iteration.
return true;
}

return false;
});
}

removeFilter({ params: { filterName } }:{ params:{ filterName:string } }) {
const filterToRemove = this.findTargetByName(filterName, this.filterTargets);
filterToRemove?.classList.add('hidden');
Expand Down
30 changes: 18 additions & 12 deletions spec/features/projects/projects_index_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -877,42 +877,45 @@ def load_and_open_filters(user)

# switching to multiselect keeps the current selection
cf_filter = page.find("li[data-filter-name='#{list_custom_field.column_name}']")

select_value_id = "#{list_custom_field.column_name}_value"

within(cf_filter) do
# Initial filter is a 'single select'
expect(cf_filter.find(:select, "value")).not_to be_multiple
expect(cf_filter.find(:select, select_value_id)).not_to be_multiple
click_on "Toggle multiselect"
# switching to multiselect keeps the current selection
expect(cf_filter.find(:select, "value")).to be_multiple
expect(cf_filter).to have_select("value", selected: list_custom_field.possible_values[2].value)
expect(cf_filter.find(:select, select_value_id)).to be_multiple
expect(cf_filter).to have_select(select_value_id, selected: list_custom_field.possible_values[2].value)

select list_custom_field.possible_values[3].value, from: "value"
select list_custom_field.possible_values[3].value, from: select_value_id
end
wait_for_reload

cf_filter = page.find("li[data-filter-name='#{list_custom_field.column_name}']")
within(cf_filter) do
# Query has two values for that filter, so it should show a 'multi select'.
expect(cf_filter.find(:select, "value")).to be_multiple
expect(cf_filter.find(:select, select_value_id)).to be_multiple
expect(cf_filter)
.to have_select("value",
.to have_select(select_value_id,
selected: [list_custom_field.possible_values[2].value,
list_custom_field.possible_values[3].value])

# switching to single select keeps the first selection
select list_custom_field.possible_values[1].value, from: "value"
unselect list_custom_field.possible_values[2].value, from: "value"
select list_custom_field.possible_values[1].value, from: select_value_id
unselect list_custom_field.possible_values[2].value, from: select_value_id

click_on "Toggle multiselect"
expect(cf_filter.find(:select, "value")).not_to be_multiple
expect(cf_filter).to have_select("value", selected: list_custom_field.possible_values[1].value)
expect(cf_filter).to have_no_select("value", selected: list_custom_field.possible_values[3].value)
expect(cf_filter.find(:select, select_value_id)).not_to be_multiple
expect(cf_filter).to have_select(select_value_id, selected: list_custom_field.possible_values[1].value)
expect(cf_filter).to have_no_select(select_value_id, selected: list_custom_field.possible_values[3].value)
end
wait_for_reload

cf_filter = page.find("li[data-filter-name='#{list_custom_field.column_name}']")
within(cf_filter) do
# Query has one value for that filter, so it should show a 'single select'.
expect(cf_filter.find(:select, "value")).not_to be_multiple
expect(cf_filter.find(:select, select_value_id)).not_to be_multiple
end

# CF date filter work (at least for one operator)
Expand Down Expand Up @@ -1590,6 +1593,9 @@ def load_and_open_filters(user)
expect(page).to have_select("add_filter_select")
# Filter for column is visible and can now be specified by the user
expect(page).to have_css(".advanced-filters--filter-name[for='created_at']")

# The correct filter input field has focus
expect(page.has_focus_on?(".advanced-filters--filter-value input#created_at_value")).to be(true)
end

it "adds the filter for a selected column that has a different filter mapped to its column" do
Expand Down

0 comments on commit 47ed900

Please sign in to comment.