Skip to content

Commit

Permalink
[26611] Avoid global preventDefault
Browse files Browse the repository at this point in the history
This causes the multivalue change event to fail complete in Chrome.
For consistency, I added the ngModelOptions to clarify use of
getter/setter on the model variable.

https://community.openproject.com/wp/26611
  • Loading branch information
oliverguenther committed Nov 8, 2017
1 parent 5ee98c4 commit 7802034
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
ng-if="!$ctrl.isMultiselect"
name="v[{{$ctrl.filter.id}}][]"
ng-model="$ctrl.value"
ng-model-options="{ getterSetter: true }"
ng-disabled="$ctrl.disabled"
ng-attr-id="values-{{$ctrl.filter.id}}"
class="advanced-filters--select"
Expand All @@ -17,6 +18,7 @@
ng-if="$ctrl.isMultiselect"
name="v[{{$ctrl.filter.id}}][]"
ng-model="$ctrl.value"
ng-model-options="{ getterSetter: true }"
ng-disabled="$ctrl.disabled"
ng-attr-id="values-{{$ctrl.filter.id}}"
class="advanced-filters--select"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<select
ng-if="!vm.field.isMultiselect"
ng-model="vm.field.value"
ng-model-options="{ getterSetter: true }"
class="focus-input wp-inline-edit--field inplace-edit--field -animated form--select"
wp-edit-field-requirements="vm.field.schema"
ng-options="value as (value.name || value.value) for value in vm.field.options track by value.href"
Expand All @@ -27,6 +28,7 @@
<select
ng-if="vm.field.isMultiselect"
ng-model="vm.field.value"
ng-model-options="{ getterSetter: true }"
class="focus-input wp-inline-edit--field inplace-edit--textarea -animated form--select"
wp-edit-field-requirements="vm.field.schema"
ng-options="value as (value.name || value.value) for value in vm.field.options track by value.href"
Expand Down
3 changes: 0 additions & 3 deletions frontend/app/components/wp-resizer/wp-resizer.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ export class WorkPackageResizerController {
}

private handleMouseUp(e:MouseEvent) {
e.preventDefault();
e.stopPropagation();

// Disable mouse move
window.removeEventListener('mousemove', this.mouseMoveHandler);

Expand Down
54 changes: 49 additions & 5 deletions spec/features/custom_fields/multi_value_custom_field_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def custom_value_for(str)
end

describe 'in the WP table' do
let(:edit_field) do
let(:table_edit_field) do
field = wp_table.edit_field work_package, "customField#{custom_field.id}"
field.field_type = 'select'
field
Expand All @@ -108,7 +108,7 @@ def custom_value_for(str)
columns.add custom_field.name
end

it 'should be usable in the table context' do
it 'should be usable in the table and split view context' do
# Disable hierarchies
hierarchy.disable_hierarchy
hierarchy.expect_no_hierarchies
Expand All @@ -129,17 +129,61 @@ def custom_value_for(str)
expect(page).to have_selector('.group--value', text: 'ham, onions, pineapple (1)')
expect(page).to have_selector('.group--value', text: 'ham (1)')

edit_field.activate!
sel = edit_field.input_element
table_edit_field.activate!
sel = table_edit_field.input_element

sel.unselect "pineapple"
sel.unselect "onions"

edit_field.submit_by_dashboard
table_edit_field.submit_by_dashboard

# Expect changed groups
expect(page).to have_selector('.group--value .count', count: 1)
expect(page).to have_selector('.group--value', text: 'ham (2)')

# Open split view
split_view = wp_table.open_split_view work_package
field = WorkPackageMultiSelectField.new(split_view.container, "customField#{custom_field.id}")

field.activate!
sel = field.input_element
expect(field).not_to be_multiselect

field.toggle_multiselect
expect(field).to be_multiselect
sel.unselect "ham"
field.submit_by_dashboard

# Expect none selected in split and table
field.expect_state_text '-'
table_edit_field.expect_state_text '-'

# Activate again
field.activate!
field.toggle_multiselect
expect(field).to be_multiselect
sel.select "ham"
sel.select "onions"

field.submit_by_dashboard

expect(field.display_element).to have_text('ham')
expect(field.display_element).to have_text('onions')
table_edit_field.expect_state_text 'ham, onions'

field.activate!
# Is now multiselect from the start, since multiple values enabled
expect(field).to be_multiselect
sel.select "pineapple"
sel.select "mushrooms"

field.submit_by_dashboard
expect(field.display_element).to have_text('ham')
expect(field.display_element).to have_text('onions')
expect(field.display_element).to have_text('pineapple')
expect(field.display_element).to have_text('mushrooms')

table_edit_field.expect_state_text ', ... 4'
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/support/pages/split_work_package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ def closed?
expect(page).not_to have_selector(@selector)
end

private

def container
find(@selector)
end

private

def path(tab = 'overview')
state = "#{work_package.id}/#{tab}"

Expand Down
33 changes: 33 additions & 0 deletions spec/support/work_packages/work_package_multi_select_field.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
require_relative './work_package_field'

class WorkPackageMultiSelectField < WorkPackageField

def multiselect?
field_container.has_selector?('.wp-inline-edit--toggle-multiselect .icon-minus2')
end

def toggle_multiselect
field_container.find('.wp-inline-edit--toggle-multiselect').click
end

def expect_save_button(enabled: true)
if enabled
expect(field_container).to have_no_selector("#{control_link}[disabled]")
else
expect(field_container).to have_selector("#{control_link}[disabled]")
end
end

def save!
submit_by_click
end

def field_type
'select'
end

def control_link(action = :save)
raise 'Invalid link' unless [:save, :cancel].include?(action)
".inplace-edit--control--#{action}"
end
end

0 comments on commit 7802034

Please sign in to comment.