Community
Participate
Working Groups
N20110411-2000. The new 'Branches' column works quite nicely. There are two issues: 1. Switching to N20110411-2000 using an existing workspace results in very small Branches column: 1. start new workspace using 3.7 M6 2. check out a project from CVS 3. select a file in the Package Explorer 4. Team > Show History ==> GOOD: opens without horizontal scroll bar 5. exit 6. start same workspace with N20110411-2000 7. select a file in the Package Explorer 8. Team > Show History ==> 'Branches' column very small (see 'Picture 2') 2. Out of the box the view should not get a horizontal scroll bar: 1. start new workspace with N20110411-2000 2. check out a project from CVS 3. select a file in the Package Explorer 4. Team > Show History ==> opens with horizontal scroll bar (see 'Picture 1')
Created attachment 193026 [details] Picture 1
Created attachment 193027 [details] Picture 2
Should polish this for M7 since this will end up in the N&N.
For case 1 I think it's better to make the 'Branches' column a bit wider and accept the horizontal scroll bar.
Created attachment 193032 [details] Picture 2
(In reply to comment #0) > 2. Out of the box the view should not get a horizontal scroll bar: > 1. start new workspace with N20110411-2000 > 2. check out a project from CVS > 3. select a file in the Package Explorer > 4. Team > Show History > ==> opens with horizontal scroll bar (see 'Picture 1') I'm afraid there's not much I can do about it. As long as you have multiple entries in the History view you will get a vertical scroll bar which will result in adding the horizontal bar as well. The initial weights used for the columns are fine. You can verify it by doing "Team > Show History" for a file which has a small number of revisions.
>As long as you have multiple >entries in the History view you will get a vertical scroll bar which will >result in adding the horizontal bar as well. I manually resized the columns and now I have the vertical scroll bar without the horizontal one - so it is possible to have only one. FYI: Same problem already in 3.6.
> The initial weights used for the > columns are fine. I guess you need to account for the possible vertical scroll bar size and make sure it fits.
For M7 we should at least make the 'Branches' column wider, so that it doesn't look like cheese.
Created attachment 193162 [details] FIx for case 1 When a new column with a negative width is found (the Branch column in this case) I treat widths for the rest of columns as weights. For the new column(s) I take the last valid width and use it as the weight of this column.
Created attachment 193163 [details] mylyn/context/zip
(In reply to comment #8) > I guess you need to account for the possible vertical scroll bar size and make > sure it fits. Have anyone done this? I think that using ColumnWeightData I'm not able to leave the margin for the possible vertical scroll bar. Columns will take all available space. I can use ColumnPixelData to layout columns and modify their widths according to the view's width, but I don't think it's the way to go.
> think that using ColumnWeightData I'm not able to > leave the margin for the possible vertical scroll bar. Columns will take all > available space. I can use ColumnPixelData to layout columns and modify their > widths according to the view's width, but I don't think it's the way to go. I agreed. The TableLayout should have support to reserve space for the vertical ruler if desired (filed bug 342807).
(In reply to comment #10) > Created attachment 193162 [details] [diff] > FIx for case 1 > > When a new column with a negative width is found (the Branch column in this > case) I treat widths for the rest of columns as weights. For the new column(s) > I take the last valid width and use it as the weight of this column. I don't like that patch for two reasons: 1. You destroy explicitly chosen widths for exiting columns. We should leave it to the user to choose which columns to reduce (if any). 2. It looks like you're using the last positive width for negative/new columns, which is not too good because the last one is the comment and this is most likely the biggest one.
(In reply to comment #13) > The TableLayout should have support to reserve space for the vertical > ruler if desired (filed bug 342807). Thanks Dani. I filed bug 342837 against Team to make sure we'll consume the fix in UI as soon as it's ready. The new bug is basically about case 2 from comment 0.
(In reply to comment #14) > I don't like that patch [...] You're right, this was the first idea that came to my mind, I agree it's not ideal. Here are other options I can think of: 1. Make the new column wide enough to display its title. So, instead of "B.." in Picture 2 you would get "Branches" 2. Keep ratio between existing columns untouched but reduce the space they take to 5/6th of total width. Make the new column take the remaining 1/6th. 3. Rather than using the last positive width for negative/new columns, take the smallest width. This should make the addition less intrusive. If the width is lower than the column's minimum width use the later.
(In reply to comment #16) > (In reply to comment #14) > > I don't like that patch [...] > > You're right, this was the first idea that came to my mind, I agree it's not > ideal. Here are other options I can think of: > 1. Make the new column wide enough to display its title. So, instead of "B.." > in Picture 2 you would get "Branches" > 2. Keep ratio between existing columns untouched but reduce the space they take > to 5/6th of total width. Make the new column take the remaining 1/6th. > 3. Rather than using the last positive width for negative/new columns, take the > smallest width. This should make the addition less intrusive. If the width is > lower than the column's minimum width use the later. I would simply make the new column bigger and leave everything else as is.
(In reply to comment #17) > I would simply make the new column bigger and leave everything else as is. I'm fine with setting the width for negative/new columns to, let's say 60px, but that would open the view with horizontal scroll bar. Is that ok with you? I was trying to avoid that.
(In reply to comment #18) > Is that ok with you? I Yep.
Created attachment 193364 [details] Fix for case 1, v02
The latest patch has been applied to HEAD. Available in builds >=N20110415-2000. Thanks for your feedback Dani.
Comment on attachment 193364 [details] Fix for case 1, v02 The fix makes things much worse: instead of giving weights to columns in a new History view in HEAD it assigns fixed 60 pixel to each. See attached screenshot. The correct fix should only do this if there already exist some columns (with known/stored width) and it should compute the minimal width for the new column based on the length of the column header label.
.
Created attachment 193469 [details] Picture of wrong History view columns
(In reply to comment #22) > The correct fix should only do this if there already exist some columns (with > known/stored width) My bad, I missed that case. > and it should compute the minimal width for the new column > based on the length of the column header label. Isn't that 1. from comment 16? I thought that by saying "I would simply make the new column bigger and leave everything else as is." you are fine with setting the column's width to arbitrary 60px (comment 18). Sorry if I misunderstood you.
(In reply to comment #25) > (In reply to comment #22) > > The correct fix should only do this if there already exist some columns (with > > known/stored width) > My bad, I missed that case. > > > and it should compute the minimal width for the new column > > based on the length of the column header label. > Isn't that 1. from comment 16? I thought that by saying "I would simply make > the new column bigger and leave everything else as is." you are fine with > setting the column's width to arbitrary 60px (comment 18). Sorry if I > misunderstood you. Yes, I wrote that 60px is also OK, but of course the real/correct fix would measure the column header text. This would also cover the case of new columns added in the future.
Created attachment 193479 [details] Fix for case 1, v03 + tests A better fix, this time with tests.
(In reply to comment #27) > Created attachment 193479 [details] > Fix for case 1, v03 + tests Applied to HEAD.
(In reply to comment #28) > (In reply to comment #27) > > Created attachment 193479 [details] [diff] > > Fix for case 1, v03 + tests > > Applied to HEAD. I'm sorry to be a pain, but: AFAIK the initial widths are: [-1, -1, -1, -1, -1, -1] but your code then results in onlyZeroes == true which is obviously wrong.
> I'm sorry to be a pain, but: AFAIK the initial widths are: > [-1, -1, -1, -1, -1, -1] > but your code then results in onlyZeroes == true which is obviously wrong. NOTE: I'm only questioning the code/implementation. Testing showed that it indeed "worked".
Created attachment 193565 [details] Fix for case 1, v04 + tests Renamed 'allZeroes' to 'reset' and added an extra test for the case when user hides a column on purpose (width = 0).
In HEAD, available in I20110419-0800.
Looks good now! I only added a break; to the loop, so that it exits when done.
