Opened 8 years ago

Closed 6 years ago

#13865 closed defect (wontfix)

dojox.mobile.Opener should scroll page to keep field in view

Reported by: colinsullivan Owned by: Adrian Vasiliu
Priority: high Milestone:
Component: DojoX Mobile Version: 1.7.0b1
Keywords: Cc:
Blocked By: Blocking:

Description

When selecting a field that is linked to an instance of with a dojox.mobile.Opener, it seems to me that the intention is to show the field directly above the opener once it is opened, as this is the native behavior on mobile devices.

Here is an opener test from dojox/mobile/tests/ with the input field pushed down to the bottom of the page: http://dojo.colin-sullivan.net/dojox/mobile/tests/test_Opener-DateSpinWheel-async-below.html

This is tricky because there is no more page to "scroll". I actually don't have any ideas on how to implement this. I am hoping you folks will.

Attachments (3)

test_Opener-scroll.html (2.5 KB) - added by Sebastien Brunot 7 years ago.
defect when using the opener in a ScrollableView?
13865-scrollableView-1.8.patch (5.9 KB) - added by Sebastien Brunot 7 years ago.
Fix overlay display in scrollable views for branch 1.8 (IBM CCLA).
13865-scrollableView-trunk.patch (6.6 KB) - added by Sebastien Brunot 7 years ago.
Fix overlay display in scrollable views for trunk (IBM CCLA).

Download all attachments as: .zip

Change History (17)

comment:1 Changed 7 years ago by ykami

Owner: changed from ykami to Eric Durocher
Status: newassigned

comment:2 Changed 7 years ago by Sebastien Brunot

Look like not only the page is not scrolling to keep the field in view (note that the Opener try to do that, by doing a window.scrollBy), but there is actually a bug when this is done in a ScrollableView. I've attached a test page (test_Opener-scroll.html) to reproduce this: open in on Safari iPhone, scroll the view down to the field and click it: the opener is displayed above the field, not at the bottom of the screen...

Changed 7 years ago by Sebastien Brunot

Attachment: test_Opener-scroll.html added

defect when using the opener in a ScrollableView?

Changed 7 years ago by Sebastien Brunot

Fix overlay display in scrollable views for branch 1.8 (IBM CCLA).

comment:3 Changed 7 years ago by Sebastien Brunot

I've attached a patch that fix the display of an overlay in a scrollable view (13865-scrollableView-1.8.patch) for branch 1.8. It didn't implements the requested scroll, though.

comment:4 Changed 7 years ago by Sebastien Brunot

I've attached a patch that fix the display of an overlay in a scrollable view (13865-scrollableView-trunk.patch) for trunk. It didn't implements the requested scroll, though.

Last edited 7 years ago by Sebastien Brunot (previous) (diff)

Changed 7 years ago by Sebastien Brunot

Fix overlay display in scrollable views for trunk (IBM CCLA).

comment:5 Changed 7 years ago by Adrian Vasiliu

Milestone: tbd1.9.1
Owner: changed from Eric Durocher to Adrian Vasiliu

comment:6 Changed 7 years ago by Adrian Vasiliu

sbrunot, a few remarks:

  • I see the patch for trunk relies on viewRegistry.getEnclosingScrollable() while the patch for 1.8 doesn't (instead it reimplements it). Maybe there's a reason for that, could you please clarify?
  • I would think you should override widget's destroy() to cleanup the reference to the scrollable parent that you introduced (to avoid memleaks).
  • It is for performance reasons that you store the scrollable parent in a widget field instead of recomputing it, right? Indeed, this reposition() method is called often, but maybe it would be worth checking if it really matters in practice. If the overhead is small, we could get rid of storing this reference.
  • (minor) There are some blanks instead of tabs in the patch (including in doc comments of the new private field).

comment:7 Changed 7 years ago by Sebastien Brunot

Thanks for your review Adrian. As a consequence, I've updated my fix and submitted a pull request on github: https://github.com/dojo/dojox/pull/12

Note that this does only fix the defect described in comment https://bugs.dojotoolkit.org/ticket/13865#comment:2

comment:8 in reply to:  7 Changed 7 years ago by Adrian Vasiliu

Thanks. I've made a few minor comments there. Now, since this fixes a related issue but not the issue described in this ticket, I doesn't seem appropriate to close this ticket once committing this fix. Instead, creating a new distinct ticket for the issue you fixed seems preferable. What do you think?

comment:9 Changed 7 years ago by Sebastien Brunot

I agree that this ticket shouldn't be closed. I leave it up to you to decide to create a dedicated ticket or to commit this other fix without a corresponding ticket (or a reference to the comment on this one).

comment:10 Changed 7 years ago by Adrian Vasiliu

Ok, I created #17229 for this distinct issue.

comment:11 Changed 7 years ago by Adrian Vasiliu

Milestone: 1.9.1

comment:12 in reply to:  10 Changed 7 years ago by Sebastien Brunot

Replying to Adrian:

Ok, I created #17229 for this distinct issue.

Thanks you Adrian.

comment:13 Changed 6 years ago by Adrian Vasiliu

I propose to close this ticket as wontfix, because no reasonable way to implement it (= to make a page scroll beyond its upper limit) has been found or appears realistically possible, and because the current behavior doesn't really forbid using the widget nor degrades significantly its usability. I will wait for feedback a while before doing it.

comment:14 Changed 6 years ago by Adrian Vasiliu

Resolution: wontfix
Status: assignedclosed
Note: See TracTickets for help on using tickets.