Activity Stream Phase 1 - Layout #2055
Conversation
farhanpatel
added
wip
Do Not Merge ⛔️
| return | ||
| } | ||
|
|
||
| img.getColors(CGSize(width: 50, height:50)) { colors in |
There was a problem hiding this comment.
I'm using UIImageColors here. It extracts dominant colors form images. Works pretty great. But need to handle edge cases better
| } | ||
| } | ||
|
|
||
| override func traitCollectionDidChange(previousTraitCollection: UITraitCollection?) { |
There was a problem hiding this comment.
By keeping track of the traits I can make sure that the right amount of topsites appear for all view sizes.
There was a problem hiding this comment.
Make sure to always call super whenever overriding view controller methods.
| Size classes define how many items to show per row/column. | ||
| */ | ||
| func numberOfVerticalItems(estimatedItems: Int) -> Int { | ||
| if let traits = currentTraits { |
There was a problem hiding this comment.
Nit: I think I mentioned this above but a rule of thumb I generally use for when to use if vs guard is if you're conditioning on the value usually being there, opt for guard so the code you expect to usually run is not buried inside a code branch. I guess in general it just reduces the branching complexity of the code by flattening it out.
|
You mentioned you already found a couple of bugs but I'll just note the ones I came across here just in case.
|
|
@farhanpatel @bkmunar Back to you guys! I would normally add a feedback+ flag but this PR is on 3 different bugs so I'll just say it here. |
|
@sleroux Should be ready to go! I dont think the keyboard should be hidden when scrolling AS. Imagine these cases Open new tab -> Tap url bar -> (you decide to search through AS.) ->you dont find what you are looking for. -> now you gotta tap the url again to type Although I'll leave it to bbell to decide once he gets to play with it. |
|
I cleaned up the naming a bit. Renamed Also renamed When I get to phase two I'll reevaluate the naming. According to the mocks there are places I can reuse |
| tableView.delegate = self | ||
| tableView.dataSource = self | ||
| tableView.rowHeight = UITableViewAutomaticDimension | ||
| tableView.separatorInset = UIEdgeInsetsZero |
There was a problem hiding this comment.
This only exists in ios10 https://developer.apple.com/reference/uikit/uiedgeinsets/1771772-zero
farhanpatel
force-pushed
the
as-phase-one
branch
from
6edc84f
to
9d1fafd
Compare
…cement for top sites.
farhanpatel
force-pushed
the
as-phase-one
branch
from
9d1fafd
to
72abfa8
Compare
I'd love for some feedback. I'll comment on some design decisions I made in #2039