Fix BIRT-designer, replaced XYLayout and XYLayoutEditPolicy (#1665) #1678
Conversation
switch from XYLayoutEditPolicy to ConstrainedLayoutEditPolicy (eclipse-birt#1665)
|
@merks |
| @@ -49,7 +49,7 @@ public Object getConstraint(IFigure child) { | |||
| */ | |||
| @Override | |||
| public void layout(IFigure container) { | |||
| List children = container.getChildren(); | |||
There was a problem hiding this comment.
I suggest using List<? extends IFigure> instead, to avoid the explicit cast in line 55.
|
|
||
| /** | ||
| * Constructor | ||
| * | ||
| * @param layout | ||
| */ | ||
| public TableXYLayoutEditPolicy(XYLayout layout) { | ||
| public TableXYLayoutEditPolicy(AbstractConstraintLayout layout) { |
There was a problem hiding this comment.
If the layout is not used internally, does it make sense to keep it as a constructor argument?
I also wonder if it makes sense to keep the class name, when it no longer extends the XYLayoutEditPolicy.
There was a problem hiding this comment.
Good question, my first topic was to get it runable :o)
(Now we can talk about improvements.)
There was a problem hiding this comment.
Oh yea, you are completely right, I removed the parameter.
|
I appreciate that you are being proactive about improving the code! I appreciate very much that you as a GEF expert are helping review the changes! (I'm traveling this week so it's harder to keep up.) |
There was a problem hiding this comment.
I find it difficult to review changes when there are a bunch of stacked commits. Generally it would be better, in my opinion, if you need to modify a PR's commit, that you amend it and force push it so that a PR has a single commit. I think some of the intermediate states here don't even compile and/or are not functionally complete/correct so it kind of pollutes the history with unless/misleading intermediate states.
But I don't want to hold things up with fussiness because the final changes do look correct and do appear to function well to eliminate the logged warnings.
|
Sorry comment intended on a different issue! |
Fix BIRT-designer, switch from XYLayout to AbstractConstraintLayout and switch from XYLayoutEditPolicy to ConstrainedLayoutEditPolicy (#1665)