Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🦡 [App Badges] Set badge count on activities tab bar item #694

Merged

Conversation

justinswart
Copy link
Contributor

📲 What

Sets the app's current badge count value on the activities tab bar item.

This is part of a larger body of work to bring badging to the app. Future PR's will include:

  • resetting badge count upon navigating to the activities tab.
  • making a GraphQL request to reset the count on the back-end.
  • updating the badge count when a push notification is received while the app is open.

🤔 Why

We've allowed for badges in the app for some time now but work has recently been done to support this on the back-end. This brings additional functionality to allow us to display the amount of unread activities in the app and to mark them as read via GraphQL.

🛠 How

  • On app launch set the value of AppEnvironment.current.application.applicationIconBadgeNumber which is UIApplication.shared.applicationIconBadgeNumber via our ApplicationType protocol on AppEnvironment.
  • Clamp this value at 99 and display a + if this value exceeds that.
  • Do not show a badge for 0 or less.

👀 See

0 unread 5 unread 99 unread > 99 unread
Screen Shot 2019-05-30 at 2 38 12 PM Screen Shot 2019-05-30 at 2 36 59 PM Screen Shot 2019-05-30 at 2 37 28 PM Screen Shot 2019-05-30 at 2 37 50 PM

✅ Acceptance criteria

  • Badge count reflects current value of UIApplication.shared.applicationIconBadgeNumber on app launch.
  • No badge is shown for zero or less.
  • 99+ is shown for an amount greater than 99.

…e-icon-set-badge

# Conflicts:
#	Kickstarter-iOS/ViewModels/RootViewModel.swift
#	Kickstarter-iOS/ViewModels/RootViewModelTests.swift
@justinswart
Copy link
Contributor Author

@justinswart justinswart changed the title [App Badges 🦡] Set badge count on activities tab bar item 🦡 [App Badges] Set badge count on activities tab bar item May 30, 2019
Copy link
Contributor

@dusi dusi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

Wondering about accessibility...should we have a custom a11y value with more human description than 99+ ... maybe more than 99? (The only thing that could slow this down would be localizations :trollface: ). Maybe even in a separate PR.

Screen Shot 2019-05-31 at 3 16 19 PM

@@ -375,3 +391,13 @@ extension TabBarItem: Equatable {
}
}
}

func activitiesBadgeValue() -> String? {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private?

@@ -4,7 +4,10 @@ import Prelude
import ReactiveSwift
import UIKit

public let maxBadgeValue = 99
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're only using this constant from the function bellow does it still make sense to have it here (as oppose to inside the function)?

If you think it's better to have it as a global constant how about we extract this to Constants.swift or something and maybe make it an enum?


guard count > 0 else { return nil }

let plusSign = AppEnvironment.current.application.applicationIconBadgeNumber > maxBadgeValue ? "+" : ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also looks like we're repeating some code here..how about

private func activitiesBadgeValue() -> String? {
  let applicationIconBadgeNumber = AppEnvironment.current.application.applicationIconBadgeNumber
  let count = min(applicationIconBadgeNumber, maxBadgeValue)

  guard count > 0 else { return nil }

  let plusSign = applicationIconBadgeNumber > maxBadgeValue ? "+" : ""

  return "\(count)\(plusSign)"
}

@@ -43,19 +43,3 @@ final class UIViewControllerURLTests: TestCase {
}
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for cleaning this up!

@justinswart
Copy link
Contributor Author

Wondering about accessibility...should we have a custom a11y value with more human description than 99+ ... maybe more than 99?

@dusi good point, will address a11y in a separate PR 👍

Copy link
Contributor

@dusi dusi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cheers

…e-icon-set-badge

# Conflicts:
#	Library/UIViewController+URLTests.swift
@justinswart justinswart merged commit f59ebd5 into feature-activity-badge-icon May 31, 2019
@justinswart justinswart deleted the feature-activity-badge-icon-set-badge branch May 31, 2019 23:12
@justinswart justinswart mentioned this pull request May 31, 2019
3 tasks
justinswart added a commit that referenced this pull request Jun 17, 2019
* 🦡 [App Badges] Set badge count on activities tab bar item (#694)

* Set badge value on app launch based on current applicationIconBadgeNumber

* Add MockApplication.swift

* Move constant to within function, improve coalescing

* SwiftFormat

* 🦡 [App Badges] Update badge value on push and lifecycle events (#698)

* Update and clear unseen activity count on lifecycle events, push, user session

* Update badge value on current user updated instead of session started, clear value on activities refresh

* Fix tests

* Only clear activity count when we know that one is set, set badge value while on activities tab, update user in environment

* Remove redundant withEnvironment

* Prevent infinite loop

* Camel-case test functions

* Do not update user in environment if logged out

* Add logged out test

* 🦡 [App Badges] Improve voice-over support for badge value (#706)

* Add Strings.activities_badge_value_plus

* Handle badge value correctly when voice over is enabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants