Skip to content
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.

Sign up for GitHub

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

Jump to bottom

873 full text search #1000

Merged
merged 5 commits into from Mar 13, 2019
Merged

873 full text search #1000

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
83a42ac
Update clojurescript
Mar 11, 2019
dd2880f
Full text search for applications
Mar 12, 2019
42b413b
Remove clear button
Mar 13, 2019
357140b
Fix tests
Mar 13, 2019
e5add72
Fix null prop
Mar 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/cljs/rems/application_list.cljs
View file
Open in desktop
Expand Up @@ -39,12 +39,10 @@
:class state-class}
:created {:value #(localize-time (:start %))
:sort-value :start
:header #(text :t.actions/created)
:filterable? false}
:header #(text :t.actions/created)}
:last-modified {:value #(localize-time (:last-modified %))
:sort-value :last-modified
:header #(text :t.actions/last-modified)
:filterable? false}
:header #(text :t.actions/last-modified)}
:view {:value view-button
:sortable? false
:filterable? false}})
Expand Down
139 changes: 89 additions & 50 deletions src/cljs/rems/table.cljs
View file
Open in desktop
@@ -1,8 +1,10 @@
(ns rems.table
"Generic sortable table widget"
(:require [rems.atoms :refer [sort-symbol search-symbol]]
(:require [clojure.string :as str]
[clojure.test :refer [deftest is testing]]
[hiccup-find.core :as hf]
[schema.core :as s]
[clojure.string :as str]))
[rems.atoms :refer [sort-symbol search-symbol]]))

(defn column-header [column-definitions col]
((get-in column-definitions [col :header] (constantly ""))))
Expand All @@ -29,11 +31,13 @@
(throw (js/Error. (str "No `:sort-value` or `:value` defined for column \"" col "\""))))))

(defn column-filter-value [column-definitions col item]
(let [filter-value-fn (or (get-in column-definitions [col :filter-value])
(get-in column-definitions [col :value]))]
(if filter-value-fn
(filter-value-fn item)
(throw (js/Error. (str "No `:filter-value` or `:value` defined for column \"" col "\""))))))
(if (get-in column-definitions [col :filterable?] true)
(let [filter-value-fn (or (get-in column-definitions [col :filter-value])
(get-in column-definitions [col :value]))]
(if filter-value-fn
(str (filter-value-fn item))
(throw (js/Error. (str "No `:filter-value` or `:value` defined for column \"" col "\"")))))
""))
hukka marked this conversation as resolved.
Show resolved Hide resolved

(defn- row [column-definitions columns item]
(into [:tr.action]
Expand Down Expand Up @@ -63,18 +67,60 @@
(reduce (fn [items {:keys [sort-column sort-order]}]
(apply-sorting column-definitions sort-column sort-order items)) items initial-sort))

(defn matches-filter [column-definitions col filter-value item]
(let [actual-value (str (column-filter-value column-definitions col item))]
(str/includes? (str/lower-case actual-value)
(str/lower-case filter-value))))

(defn matches-filters [column-definitions filters item]
(every? (fn [[col filter-value]] ()
(matches-filter column-definitions col filter-value item))
filters))
(defn match-filter? [column-definitions filter-word item]
(some (fn [col]
(str/includes? (str/lower-case (column-filter-value column-definitions col item))
(str/lower-case filter-word)))
hukka marked this conversation as resolved.
Show resolved Hide resolved
(keys column-definitions)))

(deftest test-match-filter
hukka marked this conversation as resolved.
Show resolved Hide resolved
(is (true? (match-filter? {:id {:value :id}
hukka marked this conversation as resolved.
Show resolved Hide resolved
:description {:value :description}}
"bar"
{:id "foo"
:description "bar"}))
"matches")
(is (not (match-filter? {:id {:value :id}
:description {:value :its-a-trap
:filter-value :description}}
"willnotmatch"
{:id "foo"
:its-a-trap "willnotmatch"
:description "bar"}))
"doesn't match with value if filter-value is provided"))

(defn match-filters? [column-definitions filters item]
(every? (fn [filter-word]
(match-filter? column-definitions filter-word item))
(str/split filters #"\s+")))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is a good place for the str/split. it should be done earlier so that filters can be a seq already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At most it could be moved one level up to apply-filters, which doesn't make much of a difference. Any higher and we are in representational logic that shouldn't really care how filter logic wants to interpret the filter commands.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think str/split belongs to the representational layer since it's parsing user input and not in the logic layer here. That's why I suggest that you move the parsing up to the border where the input callback is. Internally you can store the filters in a more optimal format for search and do less processing every call.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it might make sense to refactor this later anyway since there is very little value in having :filterable? and :sortable? separately compared to only specifying :filter-value and :sort-value where necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

:filter-value and :sort-value fall back to :value, so :filterable? and :sortable? exist to disable the default behavior.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes but that is also suspect.


(deftest test-match-filters
hukka marked this conversation as resolved.
Show resolved Hide resolved
(is (true? (match-filters? {:id {:value :id}
hukka marked this conversation as resolved.
Show resolved Hide resolved
:description {:value :its-a-trap
:filter-value :description}}
"foo bar"
{:id "foo"
:its-a-trap "willnotmatch"
:description "bar"}))
"match with filter-value")
(is (not (match-filters? {:id {:value :id}
:description {:value :its-a-trap
:filter-value :description}}
"foo bar"
{:id "foo"
:its-a-trap "bar"
:description "shouldnotmatch"}))
"doesn't match with value if filter-value is provided")
hukka marked this conversation as resolved.
Show resolved Hide resolved
(is (not (match-filters? {:id {:value :id}
:description {:value :description
:filterable? false}}
"foo shouldnotmatch"
{:id "foo"
:description "shouldnotmatch"}))
"doesn't match if filterable is false"))

(defn apply-filtering [column-definitions filters items]
(filter #(matches-filters column-definitions filters %) items))
(filter #(match-filters? column-definitions filters %) items))

(s/defschema Applicable
(s/cond-pre s/Keyword (s/pred fn?)))
Expand All @@ -94,20 +140,38 @@
(s/optional-key :sort-column) s/Keyword
(s/optional-key :sort-order) (s/enum :asc :desc)
(s/optional-key :set-sorting) Applicable}
(s/optional-key :filtering) {(s/optional-key :filters) {s/Keyword s/Str}
(s/optional-key :filtering) {(s/optional-key :filters) s/Str
(s/optional-key :show-filters) s/Bool
(s/optional-key :set-filtering) Applicable}
:id-function Applicable
:items [s/Any]
(s/optional-key :class) s/Str})

(defn- filter-view [filtering]
(let [{:keys [filters set-filtering]} filtering]
[:div
[:input
hukka marked this conversation as resolved.
Show resolved Hide resolved
{:type "text"
:name "table-search"
:value filters
:placeholder ""
:on-input (fn [event]
(set-filtering (assoc filtering :filters (-> event .-target .-value))))}]
hukka marked this conversation as resolved.
Show resolved Hide resolved
(when (seq filters)
[:div.reset-button.icon-link.fa.fa-backspace
hukka marked this conversation as resolved.
Show resolved Hide resolved
{:on-click (fn []
(set-filtering (assoc filtering :filters "")))
:aria-hidden true}])]))

(defn- filter-toggle [{:keys [show-filters set-filtering] :as filtering}]
(when filtering
[:div.rems-table-search-toggle.d-flex.flex-row-reverse
[:button.btn
{:class (if show-filters "btn-secondary" "btn-primary")
:on-click #(set-filtering (update filtering :show-filters not))}
(search-symbol)]]))
(search-symbol)]
(when show-filters
[filter-view filtering])]))

(defn- column-header-view [column column-definitions sorting]
(let [{:keys [sort-column sort-order set-sorting]} sorting
Expand All @@ -124,45 +188,20 @@
(when (= column sort-column)
(sort-symbol sort-order))]]))

(defn- column-filter-view [column column-definitions filtering]
(let [{:keys [show-filters filters set-filtering]} filtering]
[:th
(when (get-in column-definitions [column :filterable?] true)
[:div.column-filter
[:input
{:type "text"
:name (str (name column) "-search")
:value (str (column filters))
:placeholder ""
:on-input (fn [event]
(set-filtering
(assoc-in filtering [:filters column] (-> event .-target .-value))))}]
(when (not= "" (get filters column ""))
[:div.reset-button.icon-link.fa.fa-backspace
{:on-click (fn []
(set-filtering
(assoc-in filtering [:filters column] "")))
:aria-hidden true}])])]))

(defn- head [{:keys [column-definitions visible-columns sorting filtering id-function items class] :as params}]
(let [{:keys [show-filters]} filtering]
[:thead
(into [:tr]
(for [column visible-columns]
[column-header-view column column-definitions sorting]))
(when show-filters
(into [:tr]
(for [column visible-columns]
[column-filter-view column column-definitions filtering])))]))
[:thead
(into [:tr]
(for [column visible-columns]
[column-header-view column column-definitions sorting]))])

(defn- body [{:keys [column-definitions visible-columns sorting filtering id-function items class] :as params}]
(let [{:keys [initial-sort sort-column sort-order set-sorting]} sorting
{:keys [show-filters filters set-filtering]} filtering]
{:keys [show-filters filters]} filtering]
[:tbody
(map (fn [item] ^{:key (id-function item)} [row column-definitions visible-columns item])
(cond->> items
(and initial-sort (not sort-column)) (apply-initial-sorting column-definitions initial-sort)
(and filtering filters) (apply-filtering column-definitions filters)
(and show-filters filters) (apply-filtering column-definitions filters)
(and sorting sort-column) (apply-sorting column-definitions sort-column sort-order)))]))

(defn component
Expand Down