Bugzilla – Full Text Bug Listing
| Summary: | Add possibility to add a control to the head of a column | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Thomas Schindl <tom.schindl> | ||||||||||
| Component: | Nebula | Assignee: | Chris Gross <cgross> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||
| Severity: | normal | ||||||||||||
| Priority: | P3 | Flags: | tom.schindl:
review?
(cgross) |
||||||||||
| Version: | unspecified | ||||||||||||
| Target Milestone: | --- | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Mac OS X - Carbon (unsup.) | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Thomas Schindl
Created attachment 105448 [details]
Work ontop of the original one
I've take the idea from Sergey Stepanenko and reworked it to have a better API. There are still some corner cases (at least on OS-X the second column is not rendered appropiately when SWT.BORDER is used)
Created attachment 105452 [details]
Fix to recalculate the header appropiately after a control has been set
Created attachment 105637 [details]
Minimize Patch-Size
Marking for review by Chris Thoughts: Need comments everywhere. Specifically better comments on the public facing methods (or methods that will be implemented by others). At first glance, its not obvious what a 'header control' would be used for. Should point out that its typically used for filtering. The GridHeaderEditor class seems to only be used internally. A user doesn't instantiate this class as we normally would with other editors. So I would make that an internal class. Also needs to be commented and given the EPL header. Other than those small comments I think this looks good. Oh and I would add a separate snippet for it (I think this is important for all new features). Chris, I'm always doing the comment work before commit but its simply too much of work while the API is in flux. Rest assured that all public API will get the desired. GridHeaderEditor is at the public package because then it can access package-scoped methods on the Grid (the newly added method there is recalculateHeader() which has to get API if I move the class to internal) but it doesn't count as public-API because the class and its constructor are package-scoped. here too it will get a seperate one of course I do the same when coding JFace take a snippet already there play with the implementation and ... . (In reply to comment #6) > Oh and I would add a separate snippet for it (I think this is important for all > new features). > Hmm.... The package access/GridHeaderEditor thing is interesting. I really think we ought to do something so users don't get confused over this class. I think I would rather see the recalculateHeader method go public (and be called internal_recalculateHeader with warnings in the comments). I'd rather have one public method we don't want than a whole class we don't want public. Re comments - yea I figured as much but I'm always one to bug when I don't see javadoc completed ;) Chris, the class is *not* public and can't get subclassed/reached because it is package scoped, so JDT will not even let you import it whether you want or not. The real solution to this problem is that we should move the Grid-Implementation to internals and at the surface provide the public API. I see the potential that the more features we'll add the more internal_* we would get. Ah I see. I didn't notice that it wasn't public. Should have looked. Re the bigger question of moving to a separate class for internals - yes that would be good but also would increase the complexity of the code. I am hestitant to anything that increases the complexity of the code at this point. So I would hold off on doing this. That's what I also thought but for now I could get away with leaving it the way it is and with the current solution we have as less API as possible. Yup. Agreed. Created attachment 105902 [details]
Fix sorter location
released to CVS header_footer >= 20080907 |