Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#16181 closed feature (fixed)

Create a FilteredList mobile widget

Reported by: Eric Durocher Owned by: Adrian Vasiliu
Priority: undecided Milestone: 1.9
Component: DojoX Mobile Version: 1.8.1
Keywords: Cc:
Blocked By: Blocking:

Description

Widget containing SearchBox? + list.

Attachments (3)

patch16181.patch (104.4 KB) - added by Adrian Vasiliu 6 years ago.
Mixin providing filtering capabilities to all list widgets. Includes tests/demos for each use-case and a DOH test. - Adrian Vasiliu, IBM, CCLA
patch16181-fix1.patch (3.6 KB) - added by Adrian Vasiliu 6 years ago.
1/ Bug fix: ensure the items are visible after filtering a scrolled list. 2/ Improvement: restore previous scroll position when going back to unfiltered list. 3/ cleanup in a test (del unused css). - Adrian Vasiliu (IBM, CCLA)
patch16181-fix2.patch (5.2 KB) - added by Adrian Vasiliu 6 years ago.
1/ Bug fix: ensure the items are visible after filtering a scrolled list. 2/ Improvement: restore previous scroll position when going back to unfiltered list. 3/ API doc addition. 4/ cleanup in a test (del unused css). - Adrian Vasiliu (IBM, CCLA)

Download all attachments as: .zip

Change History (34)

comment:1 Changed 7 years ago by Eric Durocher

Milestone: tbd1.9
Owner: changed from Eric Durocher to Adrian Vasiliu
Status: newassigned

comment:2 Changed 6 years ago by Adrian Vasiliu

Feedback on the attached patch is (of course) welcome. And thanks to cjolif for his input on the numerous points I've already discussed with him.

Last edited 6 years ago by Adrian Vasiliu (previous) (diff)

comment:3 Changed 6 years ago by Eric Durocher

This looks very nice, just some very minor comments:

  • In the class description 'dojo/mobile' should be ' dojox/mobile'.
  • The comment in _initStore about detecting old stores does not seem consistent with the code.
  • About the implementation note on requiring ScrollableView: the reason seems a little weak, if you just wanted users to access the created scrollable view on ready you could just say that the application should require ScrollableView upfront. But I agree that the static require is not a big problem either.

comment:4 Changed 6 years ago by Adrian Vasiliu

Thanks Eric, agree with your points. I'll do the necessary changes, together with other corrections.

comment:5 Changed 6 years ago by Douglas Hays

In [30191]:

Refs #14835, #16181. Proxy commit for Adrian (IBM, CCLA). Optimize test_SearchBox-demo.html to use dojo/store/Memory instead of ItemFileReadStore?+DataStore?.

Changed 6 years ago by Adrian Vasiliu

Attachment: patch16181.patch added

Mixin providing filtering capabilities to all list widgets. Includes tests/demos for each use-case and a DOH test. - Adrian Vasiliu, IBM, CCLA

comment:6 Changed 6 years ago by Adrian Vasiliu

Attached a new version of the patch which includes the corrections for Eric's points plus fixes for other issues I've found during further testing:

  • Fix to correct the margins around the SearchBox to match RoundRectList (side-effect: now a SearchBox-compat.css is also included).
  • Adapt the DOH test to the above changes.
  • Fix to correct the margins around the SearchBox on iPad.
  • Addition of the new tests in tests/index.js.
  • Removal of tests/test_SearchBox-demo.html (now superseeded by the new tests).

comment:7 Changed 6 years ago by cjolif

In [30204]:

refs #16181. Mixin providing filtering capabilities to list widgets. Thanks Adrian Vasiliu (IBM, CCLA). !strict.

comment:8 Changed 6 years ago by cjolif

In [30205]:

refs #16181. Themes for mixin providing filtering capabilities to list widgets. Thanks Adrian Vasiliu (IBM, CCLA). !strict.

comment:9 Changed 6 years ago by cjolif

In [30206]:

refs #16181. Tests for mixin providing filtering capabilities to list widgets. Thanks Adrian Vasiliu (IBM, CCLA). !strict.

comment:10 Changed 6 years ago by cjolif

ticket will be closed once document + release notes will be written.

comment:11 Changed 6 years ago by cjolif

In [30207]:

refs #16181. Tests for mixin providing filtering capabilities to list widgets. Thanks Adrian Vasiliu (IBM, CCLA). !strict.

comment:12 Changed 6 years ago by cjolif

In [30208]:

refs #16181. Fix test reference.

comment:13 Changed 6 years ago by cjolif

In [30209]:

refs #16181. Fix test reference.

comment:14 Changed 6 years ago by cjolif

In [30210]:

refs #16181. Fix errors in previous commits

comment:15 Changed 6 years ago by cjolif

In [30214]:

refs #16181. Various small cleanups in tests. Thanks Adrian Vasiliu (IBM, CCLA)

comment:16 Changed 6 years ago by cjolif

In [30236]:

refs #16181. More cleanups. Thanks Adrian Vasiliu (IBM, CCLA)

comment:17 Changed 6 years ago by cjolif

In [30279]:

refs #16181. Add the filtered list test to the module.js list. Thanks Adrian Vasiliu (IBM, CCLA)

comment:18 Changed 6 years ago by ebengtso

I think when you use the dojox.mobile.EdgeToEdgeCategory? and there is a match on an Item of that category, the Category should be displayed together with the matching items, but only the matching items are displayed

comment:19 Changed 6 years ago by ebengtso

I´ve noticed a problem, after scrolling the view the filter will not find items

comment:20 Changed 6 years ago by Adrian Vasiliu

Thanks ebengtso for testing and for raising both points.

Concerning the issue with scrolling, I reproduced in some cases of scrolling (strangely, not always). I will fix it.

Concerning the issue with categories: in short, you hit a limitation that I will need to document. To get into details, some context: as you know, in Dojo Mobile for a list with categories you need to use one instance of a list widget per category, and to place EdgeToEdgeCategory (or RoundRectCategory) widgets as "separator" between the different list widgets. Because there are several list instances instead of just one, this design does not fit a use-case with a unique store containing the data for all the items including the category information. I would think this is the main use-case in practice for lists with categories, and, as long as this use-case is not well supported, it reduces the importance of supporting filtering with categories right now, since filtering is mostly useful for large lists, often backed by a data store.

Now, I'm not sure whether you have tried in the mode of the mixin which creates automatically the SearchBox and the ScrollableView, or in the "manual" mode (you create and place yourself the SearchBox). For sure, the "automatic" mode doesn't match the design of lists with categories, because in this mode the mixin creates a SearchBox and a ScrollableView for each individual list, which is clearly unwanted. Differently, if you create yourself the SearchBox, it can be shared by the different list widgets. Hiding the category when it becomes empty should be doable at application level using the onFilter handler of the mixin, while doing it automatically is a bit problematic since the list widgets don't have a reference to their category widget (heuristics can be implemented but they wouldn't cover all possible cases).

Last edited 6 years ago by Adrian Vasiliu (previous) (diff)

comment:21 Changed 6 years ago by ebengtso

Adrian, Thanks for the explation.

I agree with you, and to add more weight, IOS filtered lists do not get their categories displayed either.

Changed 6 years ago by Adrian Vasiliu

Attachment: patch16181-fix1.patch added

1/ Bug fix: ensure the items are visible after filtering a scrolled list. 2/ Improvement: restore previous scroll position when going back to unfiltered list. 3/ cleanup in a test (del unused css). - Adrian Vasiliu (IBM, CCLA)

comment:22 Changed 6 years ago by Adrian Vasiliu

So the attached patch fixes the issue you raised about items that can be invisible when filtering a scrolled list. Additionally, I implemented a behavior similar to iPhone's Contact app: when going back to an unfiltered list, the previous scroll position of the unfiltered list (before entering a search criteria) is restored.

And an update concerning the issue with categories: while we do provide examples where lists with categories are obtained by combining several list widgets (one per category), I realized we also provide examples of lists backed by a store with hierarchical data, where the categories are obtained thanks to ListItem's 'header' property. So I was wrong on thinking this use-case is not well supported by the store-based lists. That said, filtering such a list faces another obstacle: the new stores (dojo/store) do not support a "recursive" query. The old dojo/data stores (which are kind of deprecated) have a "deep" option for queries, allowing to match items that are children of the top-level items, and this is not supported by the current implementations of stores in dojo/store. This may change in some future... I will do the update of the doc to mention limitations after double-checking that there's indeed no good way to get it working.

Changed 6 years ago by Adrian Vasiliu

Attachment: patch16181-fix2.patch added

1/ Bug fix: ensure the items are visible after filtering a scrolled list. 2/ Improvement: restore previous scroll position when going back to unfiltered list. 3/ API doc addition. 4/ cleanup in a test (del unused css). - Adrian Vasiliu (IBM, CCLA)

comment:23 Changed 6 years ago by Adrian Vasiliu

(patch16181-fix2.patch is cumulative with the initial "fix1" - it adds an API doc sentence concerning the use of stores with hierarchical data)

comment:24 Changed 6 years ago by cjolif

In [30429]:

refs #16181. Ensure the items are visible after filtering a scrolled list. Restore previous scroll position when going back to unfiltered list. doc + cleanups. Thanks Adrian Vasiliu (IBM, CCLA)

comment:25 Changed 6 years ago by Adrian Vasiliu

I wrote: "filtering such a list [backed by a dojo/store containing hierarchical data] faces another obstacle: the new stores (dojo/store) do not support a "recursive" query".

I've entered #16599 for this issue.

comment:26 Changed 6 years ago by ben hockey

why use getFilterBox and getScrollableView rather than _getFilterBoxAttr and _getScrollableViewAttr?

comment:27 Changed 6 years ago by Adrian Vasiliu

@neonstalwart, the reasoning is the following: in this case, we don't want to expose a filterBox and a scrollableView property. The objects returned by getFilterBox and getScrollableView aren't set by the user directly. Depending on the use-case, they are either instanciated by the mixin itself, or (for the filter box) specified by the user via an id (the property filterBoxRef). If they'd be exposed as properties, the user may be tempted to set their values directly, while this is not how we intend them to use the mixin. In some extent, our getXXX are comparable with dijit's getParent, getTopPopup, getDescendants, getNextSibiling methods.

comment:28 Changed 6 years ago by Adrian Vasiliu

Reference guide and release notes are now submitted:

https://github.com/dojo/docs/pull/67

(plus https://github.com/phiggins42/rstwiki/pull/38 for CodeGlass changes to introduce support for dojox/mobile live examples).

Last edited 6 years ago by Adrian Vasiliu (previous) (diff)

comment:29 Changed 6 years ago by Adrian Vasiliu

Both pull requests have been merged, it seems the ticket can be closed.

comment:30 Changed 6 years ago by cjolif

Resolution: fixed
Status: assignedclosed

comment:31 Changed 6 years ago by Adrian Vasiliu

In [31212]:

refs #16181. Fix detection of old vs. new stores to cover cases when the (new) store does not have idProperty as own property.

Note: See TracTickets for help on using tickets.