Merge pull request #1025 from sharetribe/fix-select-multiple-filter

Fix select multiple filter

CHANGELOG.md

@@ -14,6 +14,10 @@ way to update this template, but currently, we follow a pattern:
 
 ## Upcoming version 2019-XX-XX
 
+- [fix] SelectMultipleFilter had a bug on mobile layout - `onSubmit` didn't get called. This fixes
+  also two other issues with SelectMultipleFilter: hovering on ListingCard removed dirty values on
+  repaint and there was an outline flashing on FilterForm when clicking checkboxes.
+  [#1025](https://github.com/sharetribe/flex-template-web/pull/1025)
 - [fix] Small changes in CSS files in order to match content width with the footer in pages where
   the footer is visible. Also, make side layout (used e.g. in `TermsOfServicePage`, `InboxPage`,
   `ContactDetailsPage`) align width footer better. Check responsive layouts carefulle after taking

src/components/FilterPlain/FilterPlain.js

@@ -1,5 +1,5 @@
 import React, { Component } from 'react';
-import { bool, func, node, string } from 'prop-types';
+import { bool, func, node, object, string } from 'prop-types';
 import classNames from 'classnames';
 import { injectIntl, intlShape, FormattedMessage } from 'react-intl';
 
@@ -45,6 +45,7 @@ class FilterPlainComponent extends Component {
       isSelected,
       children,
       initialValues,
+      keepDirtyOnReinitialize,
       contentPlacementOffset,
     } = this.props;
     const classes = classNames(rootClassName || css.root, className);
@@ -74,6 +75,7 @@ class FilterPlainComponent extends Component {
             contentPlacementOffset={contentPlacementOffset}
             onChange={this.handleChange}
             initialValues={initialValues}
+            keepDirtyOnReinitialize={keepDirtyOnReinitialize}
           >
             {children}
           </FilterForm>
@@ -88,6 +90,7 @@ FilterPlainComponent.defaultProps = {
   className: null,
   plainClassName: null,
   initialValues: null,
+  keepDirtyOnReinitialize: false,
 };
 
 FilterPlainComponent.propTypes = {
@@ -99,6 +102,8 @@ FilterPlainComponent.propTypes = {
   label: node.isRequired,
   isSelected: bool.isRequired,
   children: node.isRequired,
+  initialValues: object,
+  keepDirtyOnReinitialize: bool,
 
   // form injectIntl
   intl: intlShape.isRequired,

src/components/FilterPopup/FilterPopup.js

@@ -106,6 +106,7 @@ class FilterPopup extends Component {
       isSelected,
       children,
       initialValues,
+      keepDirtyOnReinitialize,
       contentPlacementOffset,
     } = this.props;
 
@@ -142,6 +143,7 @@ class FilterPopup extends Component {
                 showAsPopup
                 contentPlacementOffset={contentPlacementOffset}
                 initialValues={initialValues}
+                keepDirtyOnReinitialize={keepDirtyOnReinitialize}
                 onSubmit={this.handleSubmit}
                 onCancel={this.handleCancel}
                 onClear={this.handleClear}
@@ -161,6 +163,7 @@ FilterPopup.defaultProps = {
   className: null,
   popupClassName: null,
   initialValues: null,
+  keepDirtyOnReinitialize: false,
   contentPlacementOffset: 0,
   liveEdit: false,
   label: null,
@@ -174,6 +177,7 @@ FilterPopup.propTypes = {
   urlParam: string.isRequired,
   onSubmit: func.isRequired,
   initialValues: object,
+  keepDirtyOnReinitialize: bool,
   contentPlacementOffset: number,
   label: string.isRequired,
   isSelected: bool.isRequired,

src/components/SelectMultipleFilter/SelectMultipleFilter.css

@@ -42,7 +42,30 @@
   }
 }
 
+.fieldGroup {
+  margin: 0;
+  padding: 0;
+  border: none;
+}
+
 .fieldGroupPlain {
+  margin: 0;
   padding-left: 20px;
   padding-bottom: 30px;
+  border: none;
+}
+
+.list {
+  margin: 0;
+}
+
+.item {
+  padding: 2px 0;
+
+  /* Fix broken multi-column layout in Chrome */
+  page-break-inside: avoid;
+
+  @media (--viewportMedium) {
+    padding: 4px 0;
+  }
 }

src/components/SelectMultipleFilter/SelectMultipleFilter.js

@@ -2,11 +2,32 @@ import React, { Component } from 'react';
 import { array, arrayOf, func, number, string } from 'prop-types';
 import classNames from 'classnames';
 import { injectIntl, intlShape } from 'react-intl';
-import { FieldCheckboxGroup } from '../../components';
+import { FieldCheckbox } from '../../components';
 
 import { FilterPopup, FilterPlain } from '../../components';
 import css from './SelectMultipleFilter.css';
 
+// SelectMultipleFilter doesn't need array mutators since it doesn't require validation.
+// TODO: Live edit didn't work with FieldCheckboxGroup
+//       There's a mutation problem: formstate.dirty is not reliable with it.
+const GroupOfFieldCheckboxes = props => {
+  const { id, className, name, options } = props;
+  return (
+    <fieldset className={className}>
+      <ul className={css.list}>
+        {options.map((option, index) => {
+          const fieldId = `${id}.${option.key}`;
+          return (
+            <li key={fieldId} className={css.item}>
+              <FieldCheckbox id={fieldId} name={name} label={option.label} value={option.key} />
+            </li>
+          );
+        })}
+      </ul>
+    </fieldset>
+  );
+};
+
 class SelectMultipleFilter extends Component {
   constructor(props) {
     super(props);
@@ -99,9 +120,10 @@ class SelectMultipleFilter extends Component {
         onSubmit={handleSubmit}
         initialValues={namedInitialValues}
         urlParam={urlParam}
+        keepDirtyOnReinitialize
         {...rest}
       >
-        <FieldCheckboxGroup
+        <GroupOfFieldCheckboxes
           className={css.fieldGroup}
           name={name}
           id={`${id}-checkbox-group`}
@@ -122,7 +144,7 @@ class SelectMultipleFilter extends Component {
         urlParam={urlParam}
         {...rest}
       >
-        <FieldCheckboxGroup
+        <GroupOfFieldCheckboxes
           className={css.fieldGroupPlain}
           name={name}
           id={`${id}-checkbox-group`}

src/forms/FilterForm/FilterForm.css

@@ -1,6 +1,7 @@
 @import '../../marketplace.css';
 
 .root {
+  outline: none;
 }
 
 .contentWrapper {