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

End of Year: add stories Data Source and gestures #389

Merged
merged 19 commits into from
Oct 18, 2022

Conversation

leandroalonso
Copy link
Member

@leandroalonso leandroalonso commented Oct 13, 2022

📘 Project: #376

This PR adds a data source protocol to populate the Stories view with stories and also adds a few gestures.

Simulator.Screen.Recording.-.iPhone.14.-.2022-10-13.at.12.39.14.mp4

About the data source

I tried to be as generic as possible with how we display stories to allow different cases to be used without much effort.

In order to show stories, we need to pass a data source that conforms to StoriesDataSource. Each story is simply a View.

This makes it possible for a story to be a custom view, an image, or a video. It's also generic enough to be used for other purposes in the future.

Take a look at the TestStoriesDataSource for an example.

The overall API is still in its early days, so we'll change it as the feature gets developed.

To test

  1. Enable the endOfYear flag in FeatureFlag.swift
  2. Run the app
  3. When the prompt appears, tap "View My 2022"
  4. ✅ Check that each story is shown for 5s and that the story indicator (the rectangles at the top) are reflecting the state
  5. ✅ Tap and hold, check that the current story is "paused". The rectangle stop progressing and the story doesn't change
  6. ✅ While seeing the yellow story, tap the left portion of the screen, you should go back to the purple one
  7. ✅ While seeing the purple story, tap the right portion of the screen, you should go to the next story (yellow)
  8. ✅ Swipe down and make sure the view is dismissed ¶

¶ Swipe down can be improved with better effects (like with the view following the gesture) but this will be done in another task

Checklist

  • I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
  • I have considered adding unit tests for my changes.
  • I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

@leandroalonso leandroalonso added the [Project] End of Year 2023 End of Year project label Oct 13, 2022
@leandroalonso leandroalonso added this to the Future milestone Oct 13, 2022
@leandroalonso leandroalonso requested a review from a team as a code owner October 13, 2022 15:40
@emilylaguna emilylaguna self-assigned this Oct 13, 2022
Copy link
Contributor

@emilylaguna emilylaguna left a comment

Choose a reason for hiding this comment

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

Nice, this is shaping up! I found a bug with tapping back while on the first story (see video below).

I also left a few other comments below. Let me know what you think.

private let dataSource: StoriesDataSource
private let publisher: Timer.TimerPublisher
private var cancellable: Cancellable?
private var interval: TimeInterval = 5.seconds
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if in the future we should move this into the datasource to allow different story views to have different durations?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's the idea! I'll do it in another PR. :)

}

func previous() {
progress = Double(Int(progress) - 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

If I tap the previous action while on the first view the progress gets set to -1 which causes progress to be delayed by 5 seconds before it starts ticking up again.

storiesbug.mov

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! Fixed in e44ffee

Comment on lines 30 to 32
if currentStory >= self.numberOfStories {
newProgress = 0
self.currentStory = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about offloading this behavior to the datasource to allow it to decide how it wants to handle when we reach the end of the stories?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is also another idea I have for subsequent tasks. :)

height: value.predictedEndLocation.y - value.location.y
)

// If a quick swipe down is performed, dismiss the view
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we also planning on adding a swipe next/back gesture?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, is swipe next/back gesture common in stories inside the same "context"?

I know it's a thing when you want to "advance" (to the next user, for example), but not for the next story.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. I don't think it's needed anymore.

Base automatically changed from task/376-add-stories-screen to trunk October 13, 2022 19:26
@leandroalonso leandroalonso mentioned this pull request Oct 13, 2022
36 tasks
Copy link
Contributor

@emilylaguna emilylaguna left a comment

Choose a reason for hiding this comment

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

Nice work!

@leandroalonso leandroalonso merged commit 0e91b4b into trunk Oct 18, 2022
@leandroalonso leandroalonso deleted the task/376-add-stories-data-source branch October 18, 2022 11:49
@leandroalonso leandroalonso mentioned this pull request Oct 18, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Project] End of Year 2023 End of Year project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants