-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 Image Playground Support #23688
Add Image Playground Support #23688
Conversation
99c6485
to
6e3bc9a
Compare
Hey, @crazytonyli , I updated it and added more screens. It's ready for review. |
📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
|
|
||
let imagePlaygroundVC = ImagePlaygroundViewController() | ||
imagePlaygroundVC.delegate = controller | ||
objc_setAssociatedObject(imagePlaygroundVC, &MediaPickerMenu.strongDelegateKey, controller, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) |
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.
Just to double-check, should controller
or imagePlaygroundVC
be stored here?
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.
controller
is the value that needs to be retained. ImagePlaygroundViewController
is kept in memory by UIKit due to being presented on screen.
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.
So, this line should be ...?
objc_setAssociatedObject(imagePlaygroundVC, &MediaPickerMenu.strongDelegateKey, controller, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) | |
objc_setAssociatedObject(controller, &MediaPickerMenu.strongDelegateKey, controller, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) |
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.
The value
to be retained is a third parameter. The test is that the delegate gets called.
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.
🙈 Of course... Not sure how I misread that...
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 forget the signature literally every time I use it 😆
WordPress/Classes/ViewRelated/Media/MediaPicker/MediaPickerMenu+ImagePlayground.swift
Outdated
Show resolved
Hide resolved
5450f9c
to
873b455
Compare
…u+ImagePlayground.swift Co-authored-by: Tony Li <[email protected]>
873b455
to
03ff41c
Compare
Integrate ImagePlayground in the following screens:
Outstanding: Gravatar (asked in the respective iOS team channel)
ScreenRecording_12-16-2024.20-51-06_1.MP4
To test:
Regression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
PR submission checklist:
RELEASE-NOTES.txt
if necessary.Testing checklist: