Skip to content
This repository was archived by the owner on Jun 19, 2025. It is now read-only.

Conversation

@SergioEstevao
Copy link
Contributor

Fixes #202

Note: this PR doesn't change the position/selection indicator that will a separate PR

To test:

  • Open the picker
  • Scroll to a video cell
  • See if the video cell appearance match the specification
  • Rotate device and see if nothing strange happens
  • Select and deselect a cell and see if nothing strange happens

@SergioEstevao SergioEstevao added this to the 0.20 milestone Aug 15, 2017
@SergioEstevao SergioEstevao requested a review from frosty August 15, 2017 20:33
- (void)setCaption:(NSString *)caption
{
self.captionLabel.hidden = !(caption.length > 0);
BOOL hide = !(caption.length > 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you just do caption.length <= 0?

_captionLabel.backgroundColor = [UIColor colorWithWhite:0.2 alpha:0.7];
CGFloat labelTextSize = 12.0;
CGFloat labelHeight = 30.0;
CGFloat labelMargin = 5.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think according to the designs the label should be 10pt from the right edge and bottom, not 5.

Copy link
Contributor

@frosty frosty left a comment

Choose a reason for hiding this comment

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

Looks great, @SergioEstevao! The only change I think is needed to match the designs is that the label should be 10pt from the edges?

@SergioEstevao
Copy link
Contributor Author

@frosty update according to your comments.

Copy link
Contributor

@frosty frosty left a comment

Choose a reason for hiding this comment

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

:shipit:!

@SergioEstevao SergioEstevao merged commit 0036752 into develop Aug 16, 2017
@SergioEstevao SergioEstevao deleted the issue/202_update_video_cell_appearance branch August 16, 2017 10:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants