Opened 7 years ago

Closed 7 years ago

#15779 closed defect (fixed)

Container.scrollable not working

Reported by: Micha Owned by: Ed Chatelain
Priority: high Milestone: 1.8.2
Component: DojoX App Version: 1.8.0rc1
Keywords: Cc:
Blocked By: Blocking:

Description

This is not working with dojo 1.8.0rc1:

<div data-dojo-type="dojox/app/widgets/Container" region="center" data-dojo-props="scrollable: true">

The problem seems to be, that the scrollable property is now a boolean instead of a string (might be a side effect of switching to the data-dojo-props property?).

if(this.scrollable && (this.scrollable == "true")){

has to be (in my opinion)

if(this.scrollable){

After I corrected two occurrences in dojox.app.widgets.Container the scrollable view is applied.

Then another problem occurred:

this.domNode.style.position = "absolute";

This statement in dojox.app.widgets.Container's buildRendering method destroys my layout so that the whole view disappeared (I had comment this statement).

I also applied the latest dojox.app sources from github => same behaviour.

Change History (6)

comment:1 Changed 7 years ago by bill

Component: GeneralDojoX App
Owner: set to Ed Chatelain
Summary: dojox.app scrollable not workingContainer.scrollable not working

Sounds like you are right, I don't know why that code was there in the first place... well I guess it's because _ScrollableMixin.js is missing the line:

scrollable: true

to tell the parser that it's a Boolean. (Not needed when using data-dojo-props, but needed if attribute values are specified directly, ie scrollable=true.)

comment:2 Changed 7 years ago by Micha

Just a post note regarding the style.position problem: It seems that my safari isn't able to render to style.height = "100%" if the positioning is absolute. Perhaps I'm missing something in my layout? Is there any requirement that I might miss? I guess this is not a bug in dojox.app.

comment:3 Changed 7 years ago by Ed Chatelain

Well I am not sure why it was coded this way, but now there are apps which have set scrollable to true (boolean) which will be broken if we change this in 1.8. The demos/todoApp has a problem because when the item list is actually inside a scrollable pane it impacts where a widget should be placed with placeAt... And of course those styles will be added when they would not have been previously.

So I think we should fix this in 1.9, but not in 1.8. In 1.8 the work around is to use data-dojo-props="scrollable: 'true'"

comment:4 Changed 7 years ago by Ed Chatelain

After working a lot on the dojox/app scrolling code I think we should fix this bug both in 1.8 and 1.9. I think it would be better to get applications doing this correctly as soon as possible. And the behavior on 1.8 is flat broken if scrollable is set to a boolean true and it does not make it scrollable. So I will make the change for 1.8 and 1.9 and I will make a note in this ticket and in the Release notes about what may be broken by the change. I will also fix the todoApp demo to work with this change.

comment:5 Changed 7 years ago by Ed Chatelain

Priority: undecidedhigh
Status: newassigned

The fix for trunk is in with this update https://github.com/dmachi/dojox_application/pull/119 The fix will go into 1.8 soon.

Note: This change may require changes to some apps, if they had set scrollable:true (boolean). For example the todoApp was using scrollable:true and it was using a programmatic widget inside that area, after this change the domNode where the programmatic widget was being added was no longer correct, and needed to be changed to avoid having fields overlay each other.

comment:6 Changed 7 years ago by Ed Chatelain

Milestone: tbd1.8.2
Resolution: fixed
Status: assignedclosed

The fix went into the 1.8 branch with this pull request: https://github.com/dmachi/dojox_application/pull/120

Note: See TracTickets for help on using tickets.