Skip to content
This repository was archived by the owner on Sep 20, 2023. It is now read-only.

Conversation

@rnystrom
Copy link
Member

Need to do some cleanup, but this looks pretty neat

simulator screen shot - iphone 8 plus - 2018-08-13 at 00 43 57

@BasThomas
Copy link
Collaborator

Love it!

Sent with GitHawk

@rnystrom
Copy link
Member Author

I'll try and sneak this in for the latest release, why not. The whole UIMenu stuff seems to break a lot of things w/ the UICollectionView anyways.

@BasThomas
Copy link
Collaborator

Go for it :)

@rnystrom
Copy link
Member Author

#yolo

@rnystrom rnystrom mentioned this pull request Aug 13, 2018
else { return }

var index = -1
for (i, model) in viewModels.reversed().enumerated() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure what you're doing here. Can't parse it from the code 😕

Copy link
Collaborator

Choose a reason for hiding this comment

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

Putting it in a separate function (+ tests) would not be a bad idea I think 😇

menu.setTargetRect(addButton.imageView?.frame ?? .zero, in: addButton)
menu.setMenuVisible(true, animated: trueUnlessReduceMotionEnabled)
@objc private func onAddButton(sender: UIView) {
// addButton.becomeFirstResponder()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be removed, or?

@Huddie
Copy link
Collaborator

Huddie commented Aug 13, 2018

Didn’t see this. Looks amazing! 🤯

Sent with GitHawk

@Huddie
Copy link
Collaborator

Huddie commented Aug 14, 2018

Was ReactionsMenuViewController added?

@rnystrom rnystrom merged commit e738618 into master Aug 14, 2018
@BasThomas BasThomas deleted the reactions-menu branch August 14, 2018 17:29
@BasThomas
Copy link
Collaborator

We should add the UIAccessibilityTraitButton for these :)

Sent with GitHawk

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants