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

Add support for custom header views #379

Merged
merged 5 commits into from
Jan 14, 2022
Merged

Conversation

frosty
Copy link
Contributor

@frosty frosty commented Jan 11, 2022

This PR adds support for custom header views in the media picker collection view. This will be used in WordPress-iOS to show an info panel at the top of the picker if a user has only provided limited access to their photo library.

Three methods have been added to the media picker delegate protocol:

  • mediaPickerController(_:configureCustomHeaderView:)
  • mediaPickerControllerReferenceSizeForCustomHeaderView(_:)
  • mediaPickerControllerShouldShowCustomHeaderView(_:)

These in combination allow a delegate to control whether a custom header view is displayed, configure the view if necessary before it's displayed, and provide a reference size which will be used to display the view at the correct size.

In order for these to have an effect, the custom header type must be registered using a new method on WPMediaPickerViewController: registerClassForCustomHeaderView(_:). The class must be a subclass of UICollectionReusableView.

I've added an option to the demo app to show an example custom header:

Simulator Screen Shot - iPhone 13 Pro - 2022-01-11 at 22 54 00 Simulator Screen Shot - iPhone 13 Pro - 2022-01-11 at 22 54 09

There are a few small caveats around the current implementation:

  • Only one header is supported, and is displayed at the top of the collection view. Headers scroll with the collection view content.
  • Because the 'capture cell' (displaying a live feed of the device's camera, allowing the user to capture an image) also uses a collection view header, it can't be used at the same time as a custom header.

To test

  • Build and run the demo app
  • Tap the + button and navigate around the picker. Ensure you don't see a special header above the regular image cells. Ensure you can select an image.
  • Dismiss the picker and tap Options in the top left. Toggle the Show Custom Header option at the bottom.
  • Dismiss the options screen and tap + to show the picker again. Navigate into an album and ensure you see the demo header at the top.
  • The demo header should have a green background, which is being overridden by the configureCustomHeaderView delegate method (the demo header class has a red background on initialization).

Copy link
Contributor

@sla8c sla8c left a comment

Choose a reason for hiding this comment

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

This LGTM!

Additional comments:

  • I tested per the steps and ran up.
  • I also changed the custom header height (s/shot below) and the custom header responds accordingly ✅

image

Copy link
Contributor

@SergioEstevao SergioEstevao left a comment

Choose a reason for hiding this comment

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

Code wise is looking good, and the behaviour works as described, but I'm bit worried about this:

Because the 'capture cell' (displaying a live feed of the device's camera, allowing the user to capture an image) also uses a collection view header, it can't be used at the same time as a custom header.

What will happen in places where we have the camera capture header active? for example in the picker for your profile photo in the WP apps?

Will we need to update that picker to hide that away?

@frosty
Copy link
Contributor Author

frosty commented Jan 13, 2022

Thanks for the comment @SergioEstevao! So in the case where a camera capture header and a custom header are enabled, only the custom header will display.

In my upcoming PR for WordPress-iOS, I've only added the permissions alert to screens that don't currently use the capture cell – Media Library +, Gutenberg, Stories. In the other areas, we could just add "Take a photo" to the action sheet that appears before displaying the media picker and not use the capture cell there any more. This may actually be clearer for the user and is would be more consistent across the app.

@frosty frosty merged commit 3074b3f into trunk Jan 14, 2022
@frosty frosty deleted the feature/custom-header-view branch January 14, 2022 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants