hint for no goal when filtering and ptr on empty list by krugerk · Pull Request #506 · beeminder/BeeSwift

krugerk · 2024-10-28T23:37:27Z

uses the common technique of setting the backgroundView of the CollectionView when no items would be displayed

fixes #226
fixes #604

new notice screen:

and now with pull-to-refresh:

and the new hint:

theospears · 2024-11-10T02:53:07Z

BeeSwift/Gallery/GalleryViewController.swift


    func numberOfSections(in collectionView: UICollectionView) -> Int {
-        return 1
+        switch (filteredGoals.isEmpty, goals.isEmpty) {


numberOfSections seems like it should be a pure function which just returns a number and doesn't have side effects like updating the background. Maybe updateGoals() is a better place for this to live?

I am not sure I understand. With this change the background is set according to the number of sections and is being set in the numberOfSections. So yeah, that method has the side effect of setting the background.
As is, updateGoals() does not seem to be an ideal spot for the change either.
I made a change; take a look

BeeSwift/Gallery/GalleryViewController.swift

krugerk · 2024-12-07T01:07:13Z

BeeSwift/Gallery/GalleryViewController.swift

    }

    func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int {
        return ServiceLocator.versionManager.lastChckedUpdateState() == .UpdateRequired ? 0 : self.filteredGoals.count


include 'update required' as one of the reasons for an empty list (show need upgrade in the background view)

theospears

I agree using BackgroundView on CollectionView is a reasonable way to handle this.

This PR will conflict with #542. Given you opened it first I don't want to put the work on fixing all the merge conflicts on you, so added some feedback about what updates are necessary for this to be merged. If you're happy to follow up on them in the next week I'll hold off on #542 and fix it up to be compatible after this is merged.

theospears · 2024-12-27T04:12:13Z

BeeSwift/Gallery/GalleryViewController.swift


    func numberOfSections(in collectionView: UICollectionView) -> Int {
-        return 1
+        [filteredGoals.isEmpty, goals.isEmpty].contains(true) ? 0 : 1


Suggested change

[filteredGoals.isEmpty, goals.isEmpty].contains(true) ? 0 : 1

filteredGoals.isEmpty ? 0 : 1

If goals is empty then filtered goals will always be, right?

Yes, if goals is empty, then any subset of it would also be empty, including the one known as filteredGoals.

I see what you mean: we are always using the combination of the filter applied to the goals and thus the set in filteredGoals. When the searchBar.text the filtered is considered off and all goals are in filteredGoals.

Perhaps these names are unclear. Something like allGoals and visibleGoals might have been a better choice?

BeeSwift/Gallery/GalleryViewController.swift

theospears · 2024-12-27T04:16:52Z

BeeSwift/Gallery/GalleryViewController.swift

-    var goals : [Goal] = []
-    var filteredGoals : [Goal] = []
+    var goals : [Goal] = [] {
+        didSet {


[Do] Move this logic (and filteredGoals.didSet) to didUpdateGoals which contains all the other logic/view invalidation which is run when the set of goals update.

theospears · 2024-12-27T04:20:16Z

BeeSwift/Gallery/GalleryViewController.swift

+        var view: UIView? {
+            switch (filteredGoals.isEmpty, goals.isEmpty) {
+            case (true, false):
+                return self.makeViewForEmptyCollection(when: .noGoalsMatchingFilter)


[Do] Remove the enum and the multiple views, just pass in a string here.

I've marked this as a Do, but I'm open to being convinced otherwise. At the moment I don't see the benefit to justify the additional complexity.

Currently collectionView?.backgroundView is being set to a UIView which is just a container, containing a BSLabel with the reason no goals are shown, and, below it, another UIView acting as a spacer.

The intent was to show the text/label "above the fold" for better visibility and to not have it sit in the middle, which would be covered by the MBProgressHUD activity indicator.

Your proposal is just to set the backgroundView to the label directly?

I'm happy either to set it to the view, or the label directly. I just don't think we get much from having the NoGoalReason enum - makeViewForEmptyCollection could just take the string to show.

krugerk · 2024-12-29T22:27:12Z

I agree using BackgroundView on CollectionView is a reasonable way to handle this.

This PR will conflict with #542. Given you opened it first I don't want to put the work on fixing all the merge conflicts on you, so added some feedback about what updates are necessary for this to be merged. If you're happy to follow up on them in the next week I'll hold off on #542 and fix it up to be compatible after this is merged.

Thanks for the consideration. (some amount of) Updating the MR so that it is mergable with its target is to be expected. And I would update it accordingly.

BeeSwift/Gallery/GalleryViewController.swift

krugerk · 2024-12-30T00:59:26Z

BeeSwift/Gallery/GalleryViewController.swift

    }

    func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) {
        let row = (indexPath as NSIndexPath).row


Suggested change

let row = (indexPath as NSIndexPath).row

let row = indexPath.row

krugerk · 2024-12-30T00:59:50Z

BeeSwift/Gallery/GalleryViewController.swift

    func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) {
        let row = (indexPath as NSIndexPath).row
-        if row < self.filteredGoals.count { self.openGoal(self.filteredGoals[row]) }
+        if row < self.visibleGoals.count { self.openGoal(self.visibleGoals[row]) }


Suggested change

if row < self.visibleGoals.count { self.openGoal(self.visibleGoals[row]) }

if row < self.visibleGoals.count {

self.openGoal(self.visibleGoals[row])

}

krugerk · 2024-12-30T11:05:37Z

BeeSwift/Gallery/GalleryViewController.swift

+            matchingGoal = self.allGoals.filter({ (goal) -> Bool in
                return goal.slug == slug
            }).last


Suggested change

matchingGoal = self.allGoals.filter({ (goal) -> Bool in

return goal.slug == slug

}).last

matchingGoal = self.allGoals.last(where: { $0.slug == slug })

unrelated to this PR though

@objc func openGoalFromNotification(_ notification: Notification) { var goalFromIdentifier: Goal? { guard let identifier = notification.userInfo?["identifier"] as? String, let url = URL(string: identifier), let objectID = viewContext.persistentStoreCoordinator?.managedObjectID(forURIRepresentation: url) else { return nil } return viewContext.object(with: objectID) as? Goal } var goalFromSlug: Goal? { guard let slug = notification.userInfo?["slug"] as? String else { return nil } return self.allGoals.last(where: { $0.slug == slug }) } if let matchingGoal = goalFromIdentifier ?? goalFromSlug { self.navigationController?.popToRootViewController(animated: false) self.openGoal(matchingGoal) } }

krugerk · 2024-12-30T11:07:59Z

BeeSwift/Gallery/GalleryViewController.swift

+        case (true, false):
+            return self.makeViewForEmptyCollection(when: .noGoalsMatchingFilter)
+        case (true, true):
+            return self.makeViewForEmptyCollection(when: .noActiveGoals)
+        default:
+            return nil


Suggested change

case (true, false):

return self.makeViewForEmptyCollection(when: .noGoalsMatchingFilter)

case (true, true):

return self.makeViewForEmptyCollection(when: .noActiveGoals)

default:

return nil

case (true, false):

self.makeViewForEmptyCollection(when: .noGoalsMatchingFilter)

case (true, true):

self.makeViewForEmptyCollection(when: .noActiveGoals)

default:

nil

krugerk · 2024-12-30T11:09:14Z

BeeSwift/Gallery/GalleryViewController.swift

        let cell:GoalCollectionViewCell = self.collectionView!.dequeueReusableCell(withReuseIdentifier: self.cellReuseIdentifier, for: indexPath) as! GoalCollectionViewCell

-        let goal:Goal = self.filteredGoals[(indexPath as NSIndexPath).row]
+        let goal:Goal = self.visibleGoals[(indexPath as NSIndexPath).row]


Suggested change

let goal:Goal = self.visibleGoals[(indexPath as NSIndexPath).row]

let goal:Goal = self.visibleGoals[indexPath.row]

krugerk · 2024-12-30T11:09:53Z

BeeSwift/Gallery/GalleryViewController.swift

@@ -429,20 +416,20 @@ class GalleryViewController: UIViewController, UICollectionViewDelegateFlowLayou
    func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
        let cell:GoalCollectionViewCell = self.collectionView!.dequeueReusableCell(withReuseIdentifier: self.cellReuseIdentifier, for: indexPath) as! GoalCollectionViewCell


why self.collectionView with ! and not merely the collectionView passed into this method?

BeeSwift/Gallery/GalleryViewController.swift

krugerk · 2024-12-30T11:21:33Z

BeeSwift/Gallery/GalleryViewController.swift

+    var allGoals : [Goal] = []
+    var visibleGoals : [Goal] = []


Suggested change

var allGoals : [Goal] = []

var visibleGoals : [Goal] = []

var allGoals = [Goal]()

var visibleGoals = [Goal]()

krugerk · 2024-12-30T11:21:49Z

BeeSwift/Gallery/GalleryViewController.swift

    var deadbeatView = UIView()
    var outofdateView = UIView()


Suggested change

var deadbeatView = UIView()

var outofdateView = UIView()

let deadbeatView = UIView()

let outOfDateView = UIView()

uses the common technique of setting the backgroundView of the CollectionView when no items would be displayed fixes beeminder#226

…lery

krugerk · 2026-01-07T16:17:58Z

The #604 relevant parts have been made obsolete by #708.

krugerk force-pushed the bugfix/226-no-pull-to-refresh-on-empty-gallery branch from 3da3a0d to 6781c47 Compare October 29, 2024 09:19

theospears self-assigned this Nov 6, 2024

theospears reviewed Nov 10, 2024

View reviewed changes

theospears assigned krugerk and unassigned theospears Nov 10, 2024

krugerk force-pushed the bugfix/226-no-pull-to-refresh-on-empty-gallery branch 2 times, most recently from ddf0553 to 5a582e2 Compare November 15, 2024 22:38

dreeves mentioned this pull request Nov 22, 2024

No pull-to-refresh on an empty gallery #226

Closed

krugerk force-pushed the bugfix/226-no-pull-to-refresh-on-empty-gallery branch from 5a582e2 to d99fc1d Compare November 27, 2024 10:04

krugerk mentioned this pull request Dec 6, 2024

Switch the gallery view to use a StackView instead of manual layout #542

Merged

krugerk commented Dec 7, 2024

View reviewed changes

theospears requested changes Dec 27, 2024

View reviewed changes

krugerk force-pushed the bugfix/226-no-pull-to-refresh-on-empty-gallery branch from bfee8d4 to 04a0479 Compare December 30, 2024 00:57

krugerk requested a review from a team as a code owner December 30, 2024 00:57

krugerk requested review from theospears and removed request for a team December 30, 2024 00:57

krugerk commented Dec 30, 2024

View reviewed changes

BeeSwift/Gallery/GalleryViewController.swift Outdated Show resolved Hide resolved

krugerk commented Dec 30, 2024

View reviewed changes

krugerk force-pushed the bugfix/226-no-pull-to-refresh-on-empty-gallery branch from 9aabc04 to 3f5dad5 Compare December 30, 2024 11:04

krugerk commented Dec 30, 2024

View reviewed changes

BeeSwift/Gallery/GalleryViewController.swift Outdated Show resolved Hide resolved

krugerk commented Dec 30, 2024

View reviewed changes

krugerk force-pushed the bugfix/226-no-pull-to-refresh-on-empty-gallery branch from af1943c to c86c79e Compare January 12, 2025 15:30

krugerk force-pushed the bugfix/226-no-pull-to-refresh-on-empty-gallery branch from c86c79e to 9607a6c Compare March 17, 2025 13:55

hint for no goal when filtering and ptr on empty list

747fa81

uses the common technique of setting the backgroundView of the CollectionView when no items would be displayed fixes beeminder#226

krugerk force-pushed the bugfix/226-no-pull-to-refresh-on-empty-gallery branch from 9607a6c to 747fa81 Compare March 17, 2025 19:36

krugerk added 3 commits June 4, 2025 14:31

Merge branch 'master' into bugfix/226-no-pull-to-refresh-on-empty-gal…

e781ef8

…lery

Merge branch 'master' into bugfix/226-no-pull-to-refresh-on-empty-gal…

7f39f5c

…lery

Merge branch 'master' into bugfix/226-no-pull-to-refresh-on-empty-gal…

7f824e2

…lery

krugerk closed this Jan 7, 2026

krugerk deleted the bugfix/226-no-pull-to-refresh-on-empty-gallery branch January 7, 2026 16:18

krugerk mentioned this pull request Jan 7, 2026

Show filter-aware empty state when no goals match search #708

Merged

5 tasks

	[filteredGoals.isEmpty, goals.isEmpty].contains(true) ? 0 : 1
	filteredGoals.isEmpty ? 0 : 1

	let row = (indexPath as NSIndexPath).row
	let row = indexPath.row

	let goal:Goal = self.visibleGoals[(indexPath as NSIndexPath).row]
	let goal:Goal = self.visibleGoals[indexPath.row]

		@@ -429,20 +416,20 @@ class GalleryViewController: UIViewController, UICollectionViewDelegateFlowLayou
		func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
		let cell:GoalCollectionViewCell = self.collectionView!.dequeueReusableCell(withReuseIdentifier: self.cellReuseIdentifier, for: indexPath) as! GoalCollectionViewCell

Conversation

krugerk commented Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theospears left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krugerk commented Dec 29, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krugerk commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

krugerk commented Oct 28, 2024 •

edited

Loading