Opened 10 years ago

Closed 10 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 10 years ago.

Download all attachments as: .zip

Change History (6)

Changed 10 years ago by Jonathan Bond-Caron

Attachment: ComboBox_paging.patch added

comment:1 Changed 10 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 10 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 10 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 10 years ago by Jonathan Bond-Caron

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

comment:5 Changed 10 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.