Update design of video queue #282
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dfirsht
reviewed
Feb 8, 2017
| @@ -654,70 +699,60 @@ extension StreamViewController: UITableViewDelegate, UITableViewDataSource { | |||
| if isKeyboardShowing { | |||
| visualEffectView.isHidden = false | |||
| dismissView.isHidden = false | |||
| } | |||
| else { | |||
| } else { | |||
There was a problem hiding this comment.
haha surprised this hasn't come up before. I've always put it on a different line for spacing but I can see why some people prefer same line
| } | ||
| } | ||
|
|
||
| func tableView(_ tableView: UITableView, commit editingStyle: UITableViewCellEditingStyle, forRowAt indexPath: IndexPath) { | ||
| if editingStyle == .delete, indexPath.row != viewModel.currentVideoIndex { |
There was a problem hiding this comment.
ok to delete indexPath.row != viewModel.currentVideoIndex only because of the canEdit method.
| return | ||
| } | ||
| guard let currentVideoIndex = viewModel.currentVideoIndex else { return } | ||
| guard let video = viewModel.videoQueue?.remove(at: sourceIndexPath.row) else { return } |
There was a problem hiding this comment.
you can combine with above guard statement and use line breaks.
| playerView.playVideo() | ||
| } | ||
| guard viewModel.currentVideoIndex != indexPath.row else { return } | ||
| guard let currentVideoIndex = viewModel.currentVideoIndex else { return } |
# Conflicts: # iOS/Stormtrooper/Stormtrooper/Storyboards/Stream.storyboard # iOS/Stormtrooper/Stormtrooper/Views/VideoQueueTableViewCell.xib
|
Changes made and design approved. Ready to merge at your go-ahead. |
dfirsht
approved these changes
Feb 8, 2017
dfirsht
pushed a commit
that referenced
this pull request
Feb 28, 2017
* Change “UP NEXT” to “QUEUE” * Update VideoQueueTableViewCell * Add play icon to assets * Update cellFor(queueTableView:at:) * Set background color of content view * Change separator color for queue table view * Improve comments * Hide separator for current video * Change number property from Int? to String? * Make current video editable * Set selection style to none * Update highlighted color * Highlight background of current video when editing * Start counting videos from 1 instead of 0 * Swap highlighting when a new video is selected * Swap highlight when video ends * Update comments and formatting * Reset background color and separator when no longer the current video * Hide separator of previous video * Code cleanup * Update row numbering after moving a video * Reload rows to update video numbers * Remove row numbering for now * Scroll to current video when showing the queue * Add helper function to highlight/unhighlight * Set previous video when loading cells * Add support for deleting the current video * Code cleanup — use switch instead of if/else * Remove ability to delete the current video * Remove video cell numbers and play icon * Update guard statements
dfirsht
pushed a commit
that referenced
this pull request
Mar 9, 2017
* Change “UP NEXT” to “QUEUE” * Update VideoQueueTableViewCell * Add play icon to assets * Update cellFor(queueTableView:at:) * Set background color of content view * Change separator color for queue table view * Improve comments * Hide separator for current video * Change number property from Int? to String? * Make current video editable * Set selection style to none * Update highlighted color * Highlight background of current video when editing * Start counting videos from 1 instead of 0 * Swap highlighting when a new video is selected * Swap highlight when video ends * Update comments and formatting * Reset background color and separator when no longer the current video * Hide separator of previous video * Code cleanup * Update row numbering after moving a video * Reload rows to update video numbers * Remove row numbering for now * Scroll to current video when showing the queue * Add helper function to highlight/unhighlight * Set previous video when loading cells * Add support for deleting the current video * Code cleanup — use switch instead of if/else * Remove ability to delete the current video * Remove video cell numbers and play icon * Update guard statements
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request adds many of the design changes from #196. There are still some tasks remaining, which are being tracked in #281.