Modern table views by mangerlahn · Pull Request #103 · gitx/gitx

mangerlahn · 2017-10-27T15:07:16Z

This PR enables layer backing, which results in 60 FPS scrolling of the historyList. This list was also converted into a view based tableView for modernization reasons.

As a side effect, changing the pane sizes using the split view divider or by resizing the window itself is also more fluent.

…ture/smooth-scrolling

For some reason, PBGitHistoryController’s awake from nib was called very often. The stack trace its called every time a new row inside the historyList is instantiated.

tiennou

Thanks for the PR, I quite like it.
I found one regression, and I'd like to investigate the awakeFromNib issue you've found.

tiennou · 2017-11-26T14:20:53Z

+		return;
+	}
+
+	_wokeUpFromNib = YES;


😭. Okay, I'll have to investigate this…

I'll take a look at this as well. I am sure there is a better solution to this..

tiennou · 2017-11-26T14:21:59Z

 	NSArray *refs = self.objectValue.refs;
 	PBGitRef *ref = [refs objectAtIndex:index];

-	NSMutableDictionary* attributes = [self attributesForRefLabelSelected:[self isHighlighted]];


I suspect this breaks the checked-out-branch-as-orange UI label.

Thanks for the hint! The parameter of this method was never used, so I removed it. The regression was created due to a missing outlet connection to the history controller.

mangerlahn · 2017-11-27T06:34:50Z

I am glad you like it! 😊

I forgot to mention that this is not ready for merging yet. There are more table views (commit view) that can be updated. I plan to do this as well..

Also remove some unneeded cell classes

mangerlahn · 2017-12-05T15:58:44Z

Ok, I have migrated the sidebar and commit lists.. Three small things besides the awakeFromNib thing:

• Text seems to be blurry on my non-retina in the sidebar
• Fast scrolling causes layout glitches for a split second in history list
• I made the text in the history list slightly bigger.. Opinions?

…cell

mangerlahn · 2017-12-29T18:53:12Z

I ironed out the two bugs.
This leaves us with the awakeFromNib fail. It seems that awakeFromNib is probably called due to its integration with the revCells. But I have not found the issue yet

ksuther · 2017-12-30T05:09:06Z

Is it just me, or does like the vertical alignment of the text look off? It looks a little too low.

ksuther · 2017-12-30T05:14:37Z

Regarding awakeFromNib, what if you change awakeFromNib to loadView and call -[super loadView] at the top of the method? That seems like it would avoid the problem.

drawLineFromColumn:toColumn:inRect:offset:color: uses NSRectFill instead of NSBezierPath when drawing a straight line. drawCircleInRect: caches the two versions of the commit circles as NSImages and redraws the cached version rather than drawing with new bezier paths each time.

ksuther · 2017-12-30T05:55:45Z

+	frame.origin.y = (self.bounds.size.height - frame.size.height) / 2;
+	frame.size.width = self.bounds.size.width - frame.origin.x;
+
+	self.textField.frame = frame;


It's a little strange to be calling setFrame: on a view inside of drawRect:. It might be better to do this in layout or setObjectValue:. You could split drawRefsInRect: into drawRefsInRect and rectForTextField: so the roles of drawing and calculating view frames are separated.

👍edit for clarity This is about @ksuther's comment above, that re-framing while display is in progress feels strange.

Optimizations in PBGitRevisionCell drawing

- Move historyController’s awakeFromNib to loadView - Move textField layout to layout method

mangerlahn · 2017-12-30T11:56:04Z

Thank you very much for the review @ksuther !

ksuther · 2017-12-30T17:42:13Z

👍 Looks good!

I get a little bit of choppy scrolling if I try to scroll hundreds of commits at a time, not sure if there's much that can be done about that though. I'm not seeing any obvious targets in Instruments right now (text drawing is taking the fast path and table view row background drawing is taking as much time as anything else). Maybe going 100% layer-backed instead of using drawRect: in PBGitRevisionCell would help, as that would avoid having to call drawRect on the entire table view column. That's probably outside the scope of this PR though :)

tiennou · 2018-02-25T23:01:27Z

Okay, done for today 😉. I've made it "clean" against master because there were nib changes, and I fixed the branch label dragging. Haven't looked at the flickering/slowness though…

Note that I almost completely killed PBRefController (and what I left won't last), so be careful around it. This has the nice property of making the responder chain less convoluted, so maybe sometime we can switch to -validateMenuItem: instead of the wreck that is PBGitRefMenuItem.

…eature/fluent-ui # Conflicts: # Classes/Controllers/PBGitHistoryController.m

mangerlahn · 2018-04-04T07:23:01Z

I finally got around to setup an Sierra instance. The flickering seems to come from -[sourceView reloadData]. I guess there is some sort of AppKit bug, with reloading the table the second the user selected a cell, that was fixed in High Sierra.

I removed these reload statements for now. I am not sure under which circumstances they would be needed.

I assume that this is probably too easy to fix the problem, so I am going to do some deeper analysis on this.

@tiennou could you check out if this „solution“ works for you? That way I at least know that I am heading into the right direction..

# Conflicts: # Classes/Controllers/PBGitHistoryController.m # Classes/Controllers/PBGitSidebarController.m # Resources/XIBs/PBGitCommitView.xib

If the webview contains a lot of content, the frame rate during resize drops dramatically This disables the webView redraw during live resizes

tiennou · 2018-04-25T23:17:40Z

Sorry, I meant to reply yesterday after taking a look but forgot to do so. The flickering source view is gone (at least at the commit you've asked it), but it felt a little sluggish still (though it seems that master shared that behavior, so maybe arrowing through the list of refs triggers a perf breakdown; haven't investigated).

One thing I noticed (because I tried to fix the merge conflict you've now fixed) is that the box view around the Amend / Commit button is gone. I'd like to keep it, because sometime™ it will need to be hot-swapped with something else (like Merge, or Abort/Skip/Continue Rebase). It's pretty minor though.

Again, thanks for doing all that work, I like the performance improvement of the history view a lot !

(78e75b2)

mangerlahn · 2018-04-26T06:25:02Z

Wow, I messed up the merge and overwrote Elia's changes (78e75b2)

I am very sorry about that!

mangerlahn · 2018-04-26T09:54:05Z

Well, the Sierra fix turns out to be not a fix at all. When checking out a branch, the checkmark is not updated. I currently try to make reloadData work again with the sidebar/sourceList.

mangerlahn · 2018-05-06T10:58:41Z

I was able to fix that now. The sidebar is behaving as expected on 10.12 and 10.13 for me

mangerlahn · 2018-06-01T00:02:13Z

I can't think of any more changes that need to be done here. Feel free to review @tiennou!

tiennou · 2018-08-19T11:35:36Z

I just found some UI glitches. Using the table headers results in a wonky layout in the Subject column.

Also, resetting the table view to the "normal" mode results in this the first time. It will fix itself on the next branch change though…

I'm tempted to merge this as is though, since it's a minor issue…

tiennou · 2018-08-19T11:40:20Z

Also, I'm about to git reset --hard over this :

diff --git i/Classes/Views/PBGitRevisionCell.h w/Classes/Views/PBGitRevisionCell.h
index ab0053d3..2e4ab834 100644
--- i/Classes/Views/PBGitRevisionCell.h
+++ w/Classes/Views/PBGitRevisionCell.h
@@ -24,7 +24,7 @@ NS_ASSUME_NONNULL_BEGIN
 - (NSRect) rectAtIndex:(int)index;
 - (void) drawLabelAtIndex:(int)index inRect:(NSRect)rect;
 
-@property (copy) PBGitCommit* objectValue;
+@property (strong) PBGitCommit *objectValue;
 
 @end

mangerlahn · 2018-08-19T11:44:21Z

Ouch, I might be able to fix these in the evening. Sorry for that!

tiennou · 2018-09-01T11:44:26Z

Merging in ! Thank you for being patient, and thanks for the PR !

mangerlahn added 7 commits October 24, 2017 17:48

Merge branch 'split-view' of https://github.com/ksuther/gitx into fea…

d28a6f2

…ture/smooth-scrolling

Add layer backing to window

783ce67

First step in moving to a view based table view

7cfeabc

Return of commit graph in historyList

03f5775

Fix long loading of branches

ec93ac1

For some reason, PBGitHistoryController’s awake from nib was called very often. The stack trace its called every time a new row inside the historyList is instantiated.

Move menu handling to commitList

888e28a

Update .gitignore

f94b87b

mangerlahn changed the title ~~More fluent UI~~ Modern table views Oct 28, 2017

tiennou requested changes Nov 26, 2017

View reviewed changes

mangerlahn added 2 commits November 27, 2017 07:29

Remove unneeded method parameter

933c62b

Add missing outlet connection to historyCell

0fbb77c

mangerlahn added 6 commits December 1, 2017 21:49

Convert commit table views to view based

23aed81

Merge branch 'master' into feature/fluent-ui

0506e63

Improve historyView appearance

994217d

Convert sidebar to view based outline view

3d264ef

Reintroduce checkedOut checkmark to sidebar

afbdbcf

Also remove some unneeded cell classes

Reintroduce context menus on sidebar

bbaf9a7

mangerlahn added 2 commits December 29, 2017 13:02

Fix blurry sidebar by removing layer backing from the source list

a0afd07

Fix flicker during scroll by removing autolayout again from revision …

a214ea6

…cell

ksuther reviewed Dec 30, 2017

View reviewed changes

mangerlahn added 2 commits December 30, 2017 12:06

Merge pull request #1 from ksuther/feature/fluent-ui

5f1641d

Optimizations in PBGitRevisionCell drawing

Multiple smaller fixes

2dd451a

- Move historyController’s awakeFromNib to loadView - Move textField layout to layout method

tiennou added 3 commits February 25, 2018 20:16

Merge branch 'develop' into develop-fluent-ui

c523a69

Merge branch 'develop' into develop-fluent-ui

88dbf5b

Merge branch 'master' into develop-fluent-ui

00b9b37

mangerlahn added 2 commits April 2, 2018 16:52

Merge branch 'feature/fluent-ui' of github.com:mangerlahn/gitx into f…

9a70f90

…eature/fluent-ui # Conflicts: # Classes/Controllers/PBGitHistoryController.m

Fix sidebar flicker on macOS Sierra

1cf2dd9

Merge branch 'master' into feature/fluent-ui

82be2fd

# Conflicts: # Classes/Controllers/PBGitHistoryController.m # Classes/Controllers/PBGitSidebarController.m # Resources/XIBs/PBGitCommitView.xib

tiennou mentioned this pull request Apr 24, 2018

Checkout a branch or commit when double clicked #127

Closed

mangerlahn added 2 commits April 26, 2018 00:51

Layout fixes

a3c8c92

WebView won’t resize during live resize of the window

cf28148

If the webview contains a lot of content, the frame rate during resize drops dramatically This disables the webView redraw during live resizes

Restore Elia’s changes to the commit view

49ed8c8

(78e75b2)

Fix non-updating sidebar

5a5a1ac

Merge branch 'master' into feature/fluent-ui

5d88be2

tiennou and others added 4 commits August 19, 2018 16:08

Merge branch 'master' into feature/fluent-ui

75f5a7c

Fix order of commits when switching branches

4d8a07b

Fix label layout in history cell

b02d724

Merge branch 'master' into feature/fluent-ui

67def17

tiennou approved these changes Sep 1, 2018

View reviewed changes

tiennou merged commit 0378e8e into gitx:master Sep 1, 2018

claytonrcarter mentioned this pull request Sep 5, 2018

Locking up as of 0378e8e #156

Closed

Conversation

mangerlahn commented Oct 27, 2017

Uh oh!

tiennou left a comment

Choose a reason for hiding this comment

Uh oh!

tiennou Nov 26, 2017

Choose a reason for hiding this comment

Uh oh!

mangerlahn Nov 27, 2017

Choose a reason for hiding this comment

Uh oh!

tiennou Nov 26, 2017

Choose a reason for hiding this comment

Uh oh!

mangerlahn Nov 27, 2017

Choose a reason for hiding this comment

Uh oh!

mangerlahn commented Nov 27, 2017

Uh oh!

mangerlahn commented Dec 5, 2017

Uh oh!

mangerlahn commented Dec 29, 2017

Uh oh!

ksuther commented Dec 30, 2017

Uh oh!

ksuther commented Dec 30, 2017

Uh oh!

ksuther Dec 30, 2017

Choose a reason for hiding this comment

Uh oh!

tiennou Feb 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mangerlahn commented Dec 30, 2017

Uh oh!

ksuther commented Dec 30, 2017

Uh oh!

tiennou commented Feb 25, 2018

Uh oh!

mangerlahn commented Apr 4, 2018

Uh oh!

tiennou commented Apr 25, 2018

Uh oh!

mangerlahn commented Apr 26, 2018

Uh oh!

mangerlahn commented Apr 26, 2018

Uh oh!

mangerlahn commented May 6, 2018

Uh oh!

mangerlahn commented Jun 1, 2018

Uh oh!

tiennou commented Aug 19, 2018

Uh oh!

tiennou commented Aug 19, 2018

Uh oh!

mangerlahn commented Aug 19, 2018

Uh oh!

tiennou commented Sep 1, 2018

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tiennou Feb 15, 2018 •

edited

Loading