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

Conversation

@Huddie
Copy link
Collaborator

@Huddie Huddie commented Jul 31, 2018

Allowing users to choose the reaction that occurs when double tapping on a comment

UserDefaults.setDefault(reaction: reaction)
defaultReactionLabel.text = reaction.emoji
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Fine merging this as-is, but if we're going to put stuff in an extension, we might as well put it in another file. We don't really follow the trend of making extensions for everything in this project.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll just remove the extension no biggy.

let menu = UIMenuController.shared
menu.menuItems = actions.map { UIMenuItem(title: $0.0, action: $0.1) }
menu.setTargetRect(defaultReactionLabel.frame, in: setDefaultReaction)
menu.setMenuVisible(true, animated: trueUnlessReduceMotionEnabled)
Copy link
Member

Choose a reason for hiding this comment

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

Using UIMenuController to pick a selection in Settings feels really weird. Could we instead create a new UITableViewController that has all of the emoji options, and you select a cell?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rnystrom Context Menu type thing or you want a whole new view controller?

Copy link
Member

Choose a reason for hiding this comment

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

While new view controller. Like how on settings sub menus are just a simple list:

GitHawk Upload by rnystrom

@rnystrom rnystrom mentioned this pull request Aug 4, 2018
@Huddie
Copy link
Collaborator Author

Huddie commented Aug 5, 2018

This has been updated to the following! Minor wording changes were made after these screenshots (Leave this by double -> Leave this when double) either way could work, let me know quick fix. And Smiley was changed to Laugh for the emoji name.

img_1730

img_1731

@Huddie
Copy link
Collaborator Author

Huddie commented Aug 5, 2018

Cells have switched to styledTextCells to match the rest of settings page

@rnystrom
Copy link
Member

rnystrom commented Aug 5, 2018

Couple more UX things:

  • Let's change the title of the cell to "Double Tap Reaction"
  • Can we just make it part of the GitHawk section? I don't think we'll ever have more options under "Reactions"
  • Let's remove the "Leave this by double tapping..." part. If we title the option "Double Tap Reaction" that should be pretty explanatory
  • How about instead of "None" we say "Disabled" or "Off"?

@Huddie
Copy link
Collaborator Author

Huddie commented Aug 5, 2018

@rnystrom Ya I don't mind having it under GitHawk I think that the issues I ref above just spoke about the fact that settings arent really broken up that much and it could use a few more sections to make it easier to navigate. The GitHawk section seems to contain settings that pertain to GitHawk as an app (Report a bug, view source code, write a review) As well as settings that pertain to the specfic users app (Badge Unread, Read on Open, and if reactions, reactions) So half are about customizing your version of GitHawk half are about GitHawk itself. Maybe we break these up into 2 sections. GitHawk and Customize?

@Huddie
Copy link
Collaborator Author

Huddie commented Aug 5, 2018

Ill shift it into GitHawk section and I guess breaking the section up can become an issue or really it should pick up from the already created issue that discusses it

Migrated back into GitHawk Section
Removed None and replaced it with a switch
@Huddie
Copy link
Collaborator Author

Huddie commented Aug 5, 2018

@rnystrom Okay those UX fixes + I changed how none/disabled enabled worked.

@Huddie
Copy link
Collaborator Author

Huddie commented Aug 5, 2018

New enable/disable UI When enabled by HuddieWhen disabled by Huddie

Sent with GitHawk

@Huddie
Copy link
Collaborator Author

Huddie commented Aug 10, 2018

Also addresses #1970

@rnystrom
Copy link
Member

Looks awesome! I'm about to cut + ship the latest version. Going to hold on landing this until we ship so we have enough time to dogfood this.

@Huddie
Copy link
Collaborator Author

Huddie commented Aug 10, 2018

Awwww man! Haha sounds good. Looking forward to the next release!

@BasThomas
Copy link
Collaborator

Looks amazing @Huddie! 😍

@Huddie
Copy link
Collaborator Author

Huddie commented Aug 10, 2018

Thanks @BasThomas!

@rnystrom rnystrom merged commit af4e0f8 into GitHawkApp:master Aug 11, 2018
@rnystrom
Copy link
Member

Gonna take it and give it a whirl 😄

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.

3 participants