Opened 9 years ago

Closed 9 years ago

#11164 closed enhancement (invalid)

[cla][patch] ComboxBox Pagination Speedup

Reported by: Jonathan Bond-Caron Owned by:
Priority: high Milestone: tbd
Component: Dijit Version: 1.5.0b2
Keywords: Cc: haysmark
Blocked By: Blocking:

Description (last modified by bill)

When hitting the 'page up/page down' key, this patch:

  • makes the scrolling fast (i'm not sure why the loop was there? was this a feature?)
  • Hitting twice the 'page up/page down' key will advance the combobox to the next page.

Attachments (1)

ComboBox_paging.patch (3.8 KB) - added by Jonathan Bond-Caron 9 years ago.

Download all attachments as: .zip

Change History (6)

Changed 9 years ago by Jonathan Bond-Caron

Attachment: ComboBox_paging.patch added

comment:1 Changed 9 years ago by bill

Cc: haysmark added
Description: modified (diff)

This looks worrisome to me... it seems like the current code has provisions (the dojo.style(this.domNode, "height") call) to scroll based on the height of the popup, which is affected by the size of the browser window. That's a different measurement than the pageSize parameter, which controls how many items are fetched from the server per request. In other words, pressing page up/page down may not cause a fetch call to the database, but rather just cause scrolling.

After your change it seems like that code is gone.

The other thing is that in some corner cases the height of each row in the drop down could vary (like when they contain an image, using labelFunc), so it's hard to tell how much to scroll except by going one by one.

This code traces back to [8833].

Mark, comments?

comment:2 Changed 9 years ago by Jonathan Bond-Caron

The patch replaces the logic with dojo.window.scrollIntoView (which is native on some browsers).

As far as I could tell, the page up/page down was meant to scroll to the "next/previous" buttons.

If the patch breaks some intended behavior, most likely it can be fixed using dojo.window.scrollIntoView / reduce the amount of code to maintain. I don't think the method was available at the time in #8833.

comment:3 Changed 9 years ago by haysmark

For arbitrarily large results (say pageSize=200), the intention of page up/page down is to scroll the menu down by approximately one click inside the scroll bar. Hence, jumping to the end of the list is not quite the same thing.

comment:4 Changed 9 years ago by Jonathan Bond-Caron

Oh I see, I'll look at it some more...

comment:5 Changed 9 years ago by bill

Resolution: invalid
Status: newclosed

Thanks, please reopen if you find an improvement that works.

Note: See TracTickets for help on using tickets.