-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[NT-505] Project page creators header #930
Conversation
@@ -19,7 +19,7 @@ | |||
<autoresizingMask key="autoresizingMask" widthSizable="YES" heightSizable="YES"/> | |||
<subviews> | |||
<containerView opaque="NO" contentMode="scaleToFill" insetsLayoutMarginsFromSafeArea="NO" translatesAutoresizingMaskIntoConstraints="NO" id="tIm-aj-d8Q"> | |||
<rect key="frame" x="0.0" y="52" width="400" height="1348"/> | |||
<rect key="frame" x="0.0" y="55" width="400" height="1346"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to move the ProjectPamphletContent container down a little bit because it was overlapping the top cell.
SwiftFormat found issues:
Generated by 🚫 Danger |
) | ||
} | ||
|
||
final class ProjectPamphletCreatorHeaderCell: UITableViewCell, ValueCell { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still not sure about this name 🤔 Would ProjectPamphletViewProgressCell
be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple questions/comments but looks good!
|
||
private func attributedLaunchDateString(with project: Project) | ||
-> NSAttributedString? { | ||
let date = Format.date(secondsInUTC: project.dates.launchedAt, dateStyle: .long, timeStyle: .none) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to pass a timezone of UTC to this date formatter so that the date doesn't get converted to the device's timezone? 🤔
} | ||
|
||
func testLaunchDateLabelAttributedText() { | ||
let project = Project.template |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we set the .launchedAt
variable on the project to a date we've defined so we can see that it's actually displaying the correct date? Otherwise this test feels a little hand-wavey.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats a good idea!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant more something like:
// Set the date to Nov 1, 2019
let dateComponents = DateComponents()
|> \.month .~ 11
|> \.day .~ 1
|> \.year .~ 2_019
|> \.timeZone .~ TimeZone.init(secondsFromGMT: 0)
let date = AppEnvironment.current.calendar.date(from: dateComponents)
let project = Project.template |> \.dates.launchedAt .~ date.timeIntervalSince1970
...
// Assert that the date is Nov 1, 2019, which is the date you set in the "launchedAt" property
self.launchDateLabelAttributedText.assertValue("You launched this project on November 1, 2019.")
You might also want to set the locale in withEnvironment
to ensure that the date is always formatted in the same way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh I see!
@@ -4,6 +4,7 @@ import Prelude | |||
|
|||
internal final class ProjectPamphletContentDataSource: ValueCellDataSource { | |||
internal enum Section: Int { | |||
case viewProgress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd name this a little closer to the cell it represents, maybe creatorHeader
?
|
||
private enum Layout { | ||
enum Button { | ||
static let height: CGFloat = 48 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are these constants from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comes from Abstract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
self.viewModel.outputs.notifyDelegateViewProgressButtonTapped | ||
.observeForUI() | ||
.observeValues { [weak self] in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we name the argument here? We typically name them when observing the values in bindViewModel
to make it clear what the value is.
} | ||
|
||
func testLaunchDateLabelAttributedText() { | ||
let project = Project.template |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant more something like:
// Set the date to Nov 1, 2019
let dateComponents = DateComponents()
|> \.month .~ 11
|> \.day .~ 1
|> \.year .~ 2_019
|> \.timeZone .~ TimeZone.init(secondsFromGMT: 0)
let date = AppEnvironment.current.calendar.date(from: dateComponents)
let project = Project.template |> \.dates.launchedAt .~ date.timeIntervalSince1970
...
// Assert that the date is Nov 1, 2019, which is the date you set in the "launchedAt" property
self.launchDateLabelAttributedText.assertValue("You launched this project on November 1, 2019.")
You might also want to set the locale in withEnvironment
to ensure that the date is always formatted in the same way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
📲 What
Note: This PR doesn't handle the case when contributors visit the project page. This will be done in the future.
🤔 Why
This task is the first part of a series of tickets that will prevent creators of backing their own project.
![JIRA ticket](https://camo.githubusercontent.com/eed141af0b47ba3373b4201c3f564631e297289e552b9374a1717250d7cc3959/68747470733a2f2f64726970737072696e742e61746c61737369616e2e6e65742f62726f7773652f4e542d353035)
🛠 How
ProjectPamphletCreatorHeaderCell
and its viewModel.ProjectPamphletCreatorHeaderCellDelegate
to notify theProjectPamphletContentViewController
when the user taps theView progress/dashboard
button.Video
storyboard was a little off and it was preventing the cell to be presented correctly.👀 See
♿️ Accessibility
✅ Acceptance criteria
** Logged-in using the
therealnativesquad
account:Help me transform this pile of wood
and select it. The Project page should be presented with the header containing a label indicating when the project was created and a button showingView progress
if project is live orView dashboard
if project is no longer live.