GitHawk icon indicating copy to clipboard operation
GitHawk copied to clipboard

Query for label on tap in RepoIssuesVC

Open BrianLitwin opened this issue 7 years ago • 5 comments
trafficstars

Connected to #2263

Summary:

In RepositoryIssuesViewController, when a user taps a label, the label is appended to the searchBar text and the query is fetched.

Test Notes:

Manually:

  • tapping on a label adds the appropriate query to the search bar and fetches the appropriate result.
  • tapping on a label that is already contained in the query doesn't append duplicate labels
  • tapping a point beside the last label in the LabelListView is passed through and an IssuesVC is pushed. Ie, the LabelListView doesn't eat touches inside it's frame where there are no label cells.

How:

There's a lot of chaining delegates involved. Reviewers: it will help to look at this view hierarchy:

  • RepositoryIssuesViewController ( BaseListViewController )
    • SearchBarSectionController ( ListSwiftSectionController )
      • SearchBarCell ( UICollectionViewCell )
    • RepositorySummarySectionController ( ListSwiftSectionController )
      • RepositorySummaryCell ( SelectableCell )
        • LabelListView ( UICollectionView )

The label is inside the LabelListView. It tells RepositorySummaryCell, which tells RepositorySummarySectionController, which tells RepositoryIssuesViewController that it has been tapped. Then: The RepoIssueViewController then tells SearchBarSectionController to tell SearchBarCell to set the searchBar text appropriately. Then the fetch proceeds as it currently would, as though the user entered the text in the searchBar. My hope is that this preserves a "one-way data flow".

BrianLitwin avatar Nov 10 '18 15:11 BrianLitwin

Manual Test Screen Vid

It's kinda hard to see but this tests the items in Test Notes above:
ezgif com-video-to-gif 4

BrianLitwin avatar Nov 10 '18 16:11 BrianLitwin

@jdisho Thanks for your review! I'm not 100% sure why this is, but I don't see the pattern of conforming to protocols in extensions used elsewhere in the app. I'm just trying to stay consistent with the rest of the codebase; I don't have any strong opinion on the issue. See IssuesViewController for an example of conforming to a lot of delegates inside the class.

BrianLitwin avatar Nov 10 '18 18:11 BrianLitwin

+1 on not using extension unless necessary. I hit compiler errors early on in GitHawk's development from over-using them and lost a ton of time debugging + refactoring.

rnystrom avatar Nov 10 '18 23:11 rnystrom

Wanted to add this screenshot with the updated method used to calculate the CollectionView's frame. See this comment above. Here, the CollectionView's border is Orange. Seems like it's working correctly:

screenshot 2018-11-11 15 27 58

BrianLitwin avatar Nov 11 '18 20:11 BrianLitwin