ScrollDecorator issues with SUnit tests and proposed code to mitigate those issues...

I have been working on a kind of Gant chart presenter.  Sometimes it needs
to be scrollable, and sometimes it does not need to be.  I am using a
ScrollDecorator.  However I noticed some undesirable behavior (relative to
what I want, at least).

The ScrollingDecoratorLayout never lets a contained view be smaller than the
ScrollDecorator that contains it.  This causes problems if my contained view
uses something like a ProportionalLayout manager.  It seems to me that when
one uses a ScrollDecorator they usually have some idea about how big they
want the contained view to be.  Sometimes it may be bigger than the
ScrollDecorator, and then it makes sense for it to have scroll bars, but
sometimes it may be smaller than the ScrollDecorator.  I don't know why that
should be prohibited.  Perhaps there could be an option to not enlarge the
contained view of a ScrollDecorator.

A package with my CJDScrollingDecoratorLayout and some SUnit tests can be
downloaded here:
http://www.mitchellscientific.com/temp/CJDScrollDecoratorIssues.zip

Look at the package comments for more test code, I was not able to implement
all issues as an SUnit tests.  I think I found and fixed at least one real
bug (ScrollingDecorator when empty has scroll bars that error if used), the
rest may be subjective.

Chris

Chris

Firstly, thanks for supplying your report in this format. It made it very
quick easy for me to reproduce and understand the issue, even though I
didn't "get it" when reading your post.

You wrote in message news:c2or9u$1vramf$[hidden email]...
> I have been working on a kind of Gant chart presenter.  Sometimes it needs
> to be scrollable, and sometimes it does not need to be.  I am using a
> ScrollDecorator.  However I noticed some undesirable behavior (relative to
> what I want, at least).
>
> The ScrollingDecoratorLayout never lets a contained view be smaller than
the
> ScrollDecorator that contains it.  This causes problems if my contained
view
> uses something like a ProportionalLayout manager.  It seems to me that
when
> one uses a ScrollDecorator they usually have some idea about how big they
> want the contained view to be.  Sometimes it may be bigger than the
> ScrollDecorator, and then it makes sense for it to have scroll bars, but
> sometimes it may be smaller than the ScrollDecorator.  I don't know why
that
> should be prohibited.  Perhaps there could be an option to not enlarge the
> contained view of a ScrollDecorator.

At first glance I was inclined to agree with you - ScrollingDecorator is
apparently not supposed to resize the views it contains, at least not
according to the class comment. However this was a deliberate feature of the
original design and changing it would amount be a breaking change for
existing applications (if it were not optional). As an example try making
your suggested change in ScrollingDecoratorLayout and then opening a
ViewComposer. Notice that the arena no longer fills the available area. For
many cases resizing the child (there is usually only one) to fill the
complete available area would seem to make sense. Please try:
http://object-arts.com/Lib/Update/Dolphin/5.1/1511.st

Regards

Blair

"Blair McGlashan" <[hidden email]> wrote in message
news:c2pe2o$1sms4s$[hidden email]...
> Firstly, thanks for supplying your report in this format. It made it very
> quick easy for me to reproduce and understand the issue, even though I
> didn't "get it" when reading your post.

I am glad it was helpfull.  Sometimes a code snippet is worth a thouand
words. ;)

> > should be prohibited.  Perhaps there could be an option to not enlarge
the
> > contained view of a ScrollDecorator.
...
> complete available area would seem to make sense. Please try:
> http://object-arts.com/Lib/Update/Dolphin/5.1/1511.st

Thanks, I tried it and it does exactly what I needed.  I can understand the
need to retain the existing functionality and appreciate the new option.
However as I was playing with it, kicking the tires, I noticed that if I
resized the window as small as I could I got an "Index out of bounds" error.
You can duplicate the error with the following code:
=========
sd := ScrollingDecorator show.
sd topShell extent: 0@0.
=========
Also, I wonder if stretchToFit is the best name for this boolean aspect.
Would something like shouldStretchToFit be better?  I notice that quite a
few of the deprecated methods in DBConnection were renamed in favor of
conveying their boolean nature in their name.

You and I, and anyone reading c.l.s.d will know about this feature.
However, I wonder if it should be visible from the ViewComposer via a
published aspect?  That might be too much bother, but I think it is worth
considering.  Since the LayoutManager for the ScrollingDecorator is not
visible in the VC this option won't be either.

It really is a joy to work with a company that is so responsive to their
customers.  That is one of the great values of Dolphin Smalltalk.

Chris

"Blair McGlashan" <[hidden email]> wrote in message
news:c2pe2o$1sms4s$[hidden email]...
...
> complete available area would seem to make sense. Please try:
> http://object-arts.com/Lib/Update/Dolphin/5.1/1511.st

I am not concerned about this (since I always put my own container in
ScrollingDecorators), but thought it might interest you since apparently it
supports multiple views.  If one adds multiple views directly to the
ScrollingDecorator the height of the last one gets enlarged ridiculously to
something like 32767 in my example.  This happens with both the old code and
the new code.  You can evaluate the code bellow as a test:
=============
sd := ScrollingDecorator show.
sd layoutManager stretchToFit: false.
sd addSubView: (ContainerView new backcolor: Color yellow; yourself).
sd addSubView: (ContainerView new backcolor: Color red; yourself).
sd addSubView: (ContainerView new backcolor: Color blue; yourself).
sd subViews do: [:each | each extent: 50@50].
sd layout.
"Inspect the wrong size views."
(sd subViews select: [:each | each extent ~= (50@50)]) inspect.
=============
Out of curiosity, when you ask for unit tests do you mean you want a real
TestCase subclass with test methods, or is something like the above good
enough for a simple problem?

Chris

"Christopher J. Demers" <[hidden email]> wrote in
message news:c2r539$21l6gr$[hidden email]...
> "Blair McGlashan" <[hidden email]> wrote in message
> news:c2pe2o$1sms4s$[hidden email]...
> > Firstly, thanks for supplying your report in this format. It made it
very the
> need to retain the existing functionality and appreciate the new option.
> However as I was playing with it, kicking the tires, I noticed that if I
> resized the window as small as I could I got an "Index out of bounds"
error.
> You can duplicate the error with the following code:
> =========
> sd := ScrollingDecorator show.
> sd topShell extent: 0@0.
> =========

Great thanks, thats the basis of a new SUnit!

> Also, I wonder if stretchToFit is the best name for this boolean aspect.
> Would something like shouldStretchToFit be better?  I notice that quite a
> few of the deprecated methods in DBConnection were renamed in favor of
> conveying their boolean nature in their name.

I quite often find a design tension there. The problem is that quite often
the "boolean" names seem contrived or grammatically incorrect (usually the
tense is the problem). In order not to address the latter problem one ends
up having to use one of 'is', 'should', or 'has'. Ideally I like to use the
'is' form, but often I can't think of a way to express the aspect like that.
Actually I wasn't quite sure what to call this aspect at all - often I find
a better name occurs to me later. I didn't think of 'shouldStretchToFit'
though, and that is better, although I'd still prefer an 'is' form.
#shouldStretchToFit it is for now, but I still don't think this really
conveys the fact that it expands the child of the scrolling decorator to fit
the available extent (if larger).

>
> You and I, and anyone reading c.l.s.d will know about this feature.
> However, I wonder if it should be visible from the ViewComposer via a
> published aspect?  That might be too much bother, but I think it is worth
> considering.  Since the LayoutManager for the ScrollingDecorator is not
> visible in the VC this option won't be either.

Yes, I hadn't thought of that. Actually I think the layout manager should be
visible as a "read-only" property (i.e. it cannot itself be set, but its
sub-aspects can).

>
> It really is a joy to work with a company that is so responsive to their
> customers.  That is one of the great values of Dolphin Smalltalk.

Thanks

Regards

Blair

"Christopher J. Demers" <[hidden email]> wrote in
message news:c2r8hv$206k4o$[hidden email]...
> "Blair McGlashan" <[hidden email]> wrote in message
> news:c2pe2o$1sms4s$[hidden email]...
> ...
> > complete available area would seem to make sense. Please try:
> > http://object-arts.com/Lib/Update/Dolphin/5.1/1511.st
>
> I am not concerned about this (since I always put my own container in
> ScrollingDecorators), but thought it might interest you since apparently
it
> supports multiple views.  If one adds multiple views directly to the
> ScrollingDecorator the height of the last one gets enlarged ridiculously
to
> something like 32767 in my example.  This happens with both the old code
and
> the new code. ...

I think it was designed to support multiple views, but in public it claims
to support only a single child. From the class comment: "in the current
implementation, [ScrollingDecorator] expects to have only one sub-view that
it is responsible for scrolling around". Apart from the fact that it doesn't
work correctly, as you've discovered, I don't think the multiple view
support is necessary at all, and removing the broken implementation would
simplify the class. Therefore that is what I propose to do, although I will
leave that for a future release.

>...You can evaluate the code bellow as a test:
> =============
> sd := ScrollingDecorator show.
>...etc
> =============
> Out of curiosity, when you ask for unit tests do you mean you want a real
> TestCase subclass with test methods, or is something like the above good
> enough for a simple problem?

A real test case is best, but a script that is easily converted to one is
nearly as good. When addressing an issue we have a policy of requiring a
test case that demonstrates it, although sometimes we make exceptions if the
issue is difficult or impossible to test this way, or if change is to
documentation only. This is particularly important with the patching process
because as we have discovered in the past, it is quite difficult to ensure
that all the claimed patches are present in the patch, that they work across
all supported platforms (we develop exclusively on XP, testing other OSs
under VMWare), and that they do not interfere with one another. Of course
there are then on-going regression testing benefits.

Regards

Blair