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.