Skip to main content
Participating Frequently
February 7, 2009
Question

[JIRA] Updated: (SDK-16727) ComboBox::calculatePreferredSizeFromData does not handle ItemPendingError

  • February 7, 2009
  • 6 replies
  • 1654 views

Matt,


Thanks for getting back to me.  I have created a subclass of ComboBox for auto-complete.  However, the class expects ComboBox functions to handle ItemPendingError appropriately.  My patches affect:


ComboBase:
SDK-16742: selectedIndex (public)
SDK-16725: setSelectedItem (private) (used by (set)selectedItem -- public)

ComboBox:


SDK-16728: calculatePreferredSizeFromData (protected)


From this list, you can see that it is possible to create a subclass that provides implementations that correctly handle ItemPendingError.  My issue here is one of SDK correctness (robustness) and documentation.  The documentation for ComboBox does not explain that data providers cannot throw ItemPendingError without expecting runtime errors visible to the application user.  In my case, I have created a class that implements ICollectionView and implements paging of remote data (i.e. a collection of lists).  When an index in the collection is requested that is not cached on the client side, an ItemPendingError is dispatched and the class requests the appropriate page from the remote data source.


The code changes for ItemPendingError are minimal and actually reduce the code in some cases.  That said, I have not done performance testing, which is definitely important.  Was any done at Adobe regarding handling of ItemPendingError?  I don't like the idea of suggesting to developers that we subclass ComboBox to re-write the mentioned functions.  This causes two problems, one is the amount of code duplication maintained independently of the SDK, but dependent on it.  The other is the number of developers that may desire similar functionality.  If ComboBox is not to support ItemPendingError, then perhaps another class should be added to the SDK with robust implementations of the mentioned functions.


Now, my patches for ListBase are a different, but related story:


ListBase:

SDK-16845: adjustAfterSort (private)


SDK-16895: commitSelectedIndex (mx_internal)

SDK-16846: selectItem (protected)


These patches are required when using a data provider that throws ItemPendingError with DataGrid, List, etc.  Where possible, it would take a substantial amount of code by a subclass to provide the robustness.  The adjustAfterSort function would need to be exposed to subclasses and I'd prefer to see commitSelectedIndex as protected instead of mx_internal.  So, while there is the opportunity for me to override the ComboBox functions for ItemPendingError, the same cannot be said for ListBase.


I can live without (but would prefer) ComboBox handling ItemPendingError as I can override the necessary functions, but I absolutely need the ListBase patches in the SDK.  At minimum, I'd like to see the ComboBox documentation updated and perhaps a technical note explaining what to do when the ItemPendingError robustness is required.  I'd be happy to work with you on any performance or coding style concerns and documentation.



Brian
This topic has been closed for replies.

6 replies

Participating Frequently
February 9, 2009

Alex,


I had to return a null from getItemAt instead of "loading" due to auto-complete matching typed text to "loading," which is not necessarily a valid option.  So far, I've found that DataGrid, List, and my label functions work without modification.

I've got a little more work on the correctness front before I get to performance, but I agree that some form of page adjustment is necessary.  I've found that ComboBox does not handle large pages well (poor display updating), while DataGrid does well with a page size of 100.


Brian
Participating Frequently
February 9, 2009

Sounds like you are getting things to work in
ComboBox/List/DataGrid without IPEs by returning temporary data.  For the
record, we haven’t done extensive testing with returning temporary data, but we
saw it work in simple cases.  List and DataGrid have support for nullItemRenderers
and hooks like createItemRenderer and createColumnItemRenderer.

Without having spent a lot of time thinking about it, if I were
to write an autocomplete combobox, it would have a parameter to tune its fetch
size to the page size of the server.  If someone is typing fast, no need
getting every reponse and throwing it away.  Just get 6 or 10 responses.  If
they stop and start to arrow through the dropdown, go get more.  Should save on
bandwidth costs and performance and all that.

Alex Harui

Flex SDK Developer

Participating Frequently
February 9, 2009
Alex,

I was able to refactor my collection to return "loading" instead of dispatching an ItemPendingError. Then, instead of notifying listeners of the IPE, I now dispatch a CollectionEventKind.REPLACE when the page is received. So far, the responsiveness does seem to be much better.

I'll need to do a lot more testing, but this seems to have been a really simple change that will make the IPE patches unnecessary for me. I'll update the issues accordingly once I've completed testing.

Brian
Participating Frequently
February 9, 2009
Alex,

Can you seed me specific suggestions for avoiding the need for IPE? I currently have a heavily modified version of ListCollectionView that implements paging. However, I maintained the use of IPE in getItemAt.

Personally, I don't like the try-catch paradigm. Error tend to go uncaught... Thus, the current issues I'm seeing. So, I'm definitely interested in an alternative implementation for getItemAt.

It would be helpful to be more open with details when initially rejecting a patch. Your current explanation makes sense. In return, I'll be sure to provide detailed test cases for future patches.

Since IPE is not in the plans going forward, the question now is how do I refactor my ICollectionView to not rely on ItemPendingError when a requested index is not in the local cache and can this be done with the currently shipped ComboBox / DataGrid / List without patches?

Brian
Participating Frequently
February 9, 2009

Brian, I noticed there weren’t test cases for the base
bugs for setting selectedIndex and selectedItem.  Test cases help us
determine the likelihood of others running into these bugs.  For example,
in setting selectedItem, you must have the actual instance of the item so the
item itself can’t be paged out.  It can’t be another instance
with the same values.  So what is  the scenario where you have the
item, but not the ones before it?

I put together a subclass of ComboBox that seems to suppress IPE
in the shipping ComboBox.  Maybe it can work for you or serve as a
starting point.

The patch submitted for calculatePreferredSizeFromData opted to
invalidateSize again when the data finally arrived.  I think it would be
best to pick a size for the ComboBox instead of having it resize as data
arrives.  Most auto-complete controls in Yahoo and Google don’t’
seem to resize.  Thus, it seems like another reason to have this kind of a
functionality in a subclass so you can dictate the response to the IPE w/o even
more flags and properties to control it.

I suppose we need to find a way to communicate the “culture
of development”.  We generally employ the 80/20 rule, worry more and
more about size every day, and performance is always a top priority.  Try/catch
blocks are somewhat expensive.  Worth avoiding if possible.

Another thing that we haven’t communicated well is that
IPE is falling out of favor on our team.  We’re starting to head in
the direction of suppressing IPEs in the collection source and supplying temporary
substitutes or null instead so the control can show “loading…”
or something like that.  Makes fault handling more pleasant to the user.
No Gumbo controls handle IPEs right now.

I know it sucks to work on a patch and have it rejected.
You should’ve seen how hard I got slammed first time I tried to write a
Gumbo component.  There’s lots of code and patterns to learn so we
get as much consistency as possible.  Each of your patches will be
reviewed on its own merits.  Keep plugging away, we really do appreciate
community participation.

Alex Harui

Flex SDK Developer

Participating Frequently
February 8, 2009

Matt,


Unfortunately, I have also found that replacing selectedIndex and setSelectedItem is not achievable without exposing the underlying variables _selectedIndex and _selectedItem.  Therefore, all of these patches are show stoppers with regard to use of ComboBox from the SDK.  My only other option is to replicate both ComboBase and ComboBox in a separate library with my patches applied.  I'd really rather not do that.


Brian