[haiku-development] Re: BMenuBar scrolling support

  • From: John Scipione <jscipione@xxxxxxxxx>
  • To: haiku-development@xxxxxxxxxxxxx
  • Date: Thu, 29 Dec 2011 14:03:36 -0500

On Wed, Dec 28, 2011 at 4:05 PM, Stephan Aßmus <superstippi@xxxxxx> wrote:

> I would like to give some feedback on the design decision to put this code
> directly into BMenu. IMHO this should not be done.

Thank your for the feedback, it is much appreciated.

For one thing, scrolling is already a fundamental concept in BView, it
> should just be used, so you don't need fScrollValue, since you can just use
> Bounds().LeftTop() to know how much the BMenu is scrolled. The drawing code
> may have to be changed/adapted. Ideally, the companion objects to which
> fBeginScroller and fEndScroller are supposed to point could work without
> the BMenu knowing about them. It should be possible to know the size
> requirements of the BMenu and thus the possible scrolling value range
> (which makes fScrollLimit unnecessary). Then maybe all BMenu needs to do is
> to make sure the currently highlighted item is visible inside the Bounds()
> area and to do regular BView scrolling to make it so. One nice thing is
> that the container view for the BMenu could be chosen freely, it could be a
> BScrollView or something else, like BMenuScroller.
>
I suppose it would be possible to eliminate a few of the variables I am
planning to add by calculating things at the time of a scroll instead of
storing the values ahead of time but I don't see how that really helps
unless I can't add the variables to BMenu without breaking binary
compatibility. I am using the ScollBy() method from BView to do the actual
scrolling in my class, but, I am not using the scrollbars so it does work a
little bit differently than BScrollView.

The fBeginScroller and fEndScroller variables help to keep track of whether
or not the scroller views are currently attached to the menu or not. A
BScrollView doesn't work because a scrollbar would add space in the wrong
dimension, meaning that a vertical menu would get horizontal space added to
it and a horizontal menu would get vertical space added to it. I don't want
that, I want to add space only in the same dimension as the menu, and the
BMenuScrollers give me that.


> Maybe I am missing something, but my point is basically that the BMenu
> code is already very bloated and would benefit from refactoring and putting
> different features into "inner" classes. Adding one more feature to the
> code will not improve its clarity. In this case it looks like the code is
> already outside of BMenu and can do what it is supposed to do. If the menu
> is expected to be too large for the screen, then the BMenu using code just
> needs to embed it properly, just like your original changes to the Deskbar,
> as I understand it.
>
The code in BMenu would have to be refactored in order to support the new
features, that is scrolling in a view instead of a window and scrolling a
BMenuBar as well as a regular BMenu. In my imagination the code to do
scrolling in BMenu would be cleaned up from what it is now.

I don't have to put my changes in BMenu, I could instead keep them in my
own class but that limits the availability of scrollers to my specialized
class which is not ideal.

Are you against the idea of having scrolling code in a BMenu in general or
just my specific implementation of the feature? If you are against
scrolling in BMenu in general I suppose that I could rip all the scrolling
code out of BMenu and put it in my own subclass. Then if you want to do
scrolling you can use the subclass I create (let's call it BScrollMenu for
now), if not, use a regular BMenu. I'd rather the feature be available
directly in BMenu but I am open to the idea of making it happen in a
subclass instead.

Other related posts: