From jb55@jb55.com Fri Jul 14 17:53:07 2023 Return-Path: Delivered-To: jb55 Received: from localhost ([::1]) by charon.jb55.com with LMTP id Jl/FOYaLsWSDJhAABqXyzQ (envelope-from ) for ; Fri, 14 Jul 2023 10:53:10 -0700 Return-Path: Delivered-To: jb55@jb55.com Received: from jb55.com (S010660e327dca171.vc.shawcable.net [24.84.152.187]) by jb55.com (OpenSMTPD) with ESMTPSA id 64346ac2 (TLSv1.3:AEAD-AES256-GCM-SHA384:256:NO); Fri, 14 Jul 2023 17:53:10 +0000 (UTC) Date: Fri, 14 Jul 2023 10:53:07 -0700 From: William Casarin To: Suhail Saqan Cc: patches@damus.io Subject: Re: [PATCH 1/8] add ReactionsSettingsView page and settings variables Message-ID: References: <20230713212605.81379-1-suhail.saqan@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230713212605.81379-1-suhail.saqan@gmail.com> Hey Suhail, Comments inline: On Thu, Jul 13, 2023 at 04:25:58PM -0500, Suhail Saqan wrote: >--- > damus/Models/UserSettingsStore.swift | 6 + > damus/Util/Router.swift | 7 + > damus/Views/ConfigView.swift | 4 + > .../Settings/ReactionsSettingsView.swift | 267 ++++++++++++++++++ > 4 files changed, 284 insertions(+) > create mode 100644 damus/Views/Settings/ReactionsSettingsView.swift > >diff --git a/damus/Models/UserSettingsStore.swift b/damus/Models/UserSettingsStore.swift >index 820ec7c4..850740ca 100644 >--- a/damus/Models/UserSettingsStore.swift >+++ b/damus/Models/UserSettingsStore.swift >@@ -157,6 +157,12 @@ class UserSettingsStore: ObservableObject { > > @Setting(key: "donation_percent", default_value: 0) > var donation_percent: Int >+ >+ @Setting(key: "emoji_reactions", default_value: default_emoji_reactions) >+ var emoji_reactions: [String] I believe you need UserDefaults.settings.stringArray for this setting to work. I don't think the current @Setting property wrapper supports this? >diff --git a/damus/Views/Settings/ReactionsSettingsView.swift b/damus/Views/Settings/ReactionsSettingsView.swift >new file mode 100644 >index 00000000..cfe98f64 >--- /dev/null >+++ b/damus/Views/Settings/ReactionsSettingsView.swift >@@ -0,0 +1,267 @@ >+// >+// ReactionsSettingsView.swift >+// damus >+// >+// Created by Suhail Saqan on 7/3/23. >+// >+ >+import SwiftUI >+import Combine >+ >+let default_emoji_reactions = ["🤣", "🤙", "⚡", "💜", "🔥", "😀", "😃", "😄", "🥶"] >+ >+struct ReactionsSettingsView: View { >+ @ObservedObject var settings: UserSettingsStore >+ >+ @State var new_emoji: String = "" >+ @State private var showActionButtons = false >+ >+ @Environment(\.dismiss) var dismiss >+ >+ var recommended: [String] { >+ return getMissingRecommendedEmojis(added: settings.emoji_reactions) >+ } >+ >+ var body: some View { >+ Form { >+ Section { >+ AddEmojiView(emoji: $new_emoji) >+ } header: { >+ HStack { >+ Text(NSLocalizedString("Add Emoji", comment: "Label for section for adding an emoji to the reactions list.")) >+ .font(.system(size: 18, weight: .heavy)) >+ .padding(.bottom, 5) There is only one element in this HStack, can just rmove? >+ } >+ } footer: { >+ VStack { This VStack seems to only have 1 element? We can just remove it. >+ HStack { >+ Spacer() >+ if(!new_emoji.isEmpty) { nit: if !new_emoji.isEmpty { >+ Button(NSLocalizedString("Cancel", comment: "Button to cancel out of view adding user inputted emoji.")) { >+ new_emoji = "" >+ } >+ .font(.system(size: 14, weight: .bold)) >+ .frame(width: 80, height: 30) >+ .foregroundColor(.white) >+ .background(LINEAR_GRADIENT) >+ .clipShape(Capsule()) >+ .padding(EdgeInsets(top: 15, leading: 0, bottom: 0, trailing: 0)) >+ >+ Button(NSLocalizedString("Add", comment: "Button to confirm adding user inputted emoji.")) { >+ if (isValidEmoji(new_emoji)) { nit: if isValidEmoji(new_emoji) { >+ settings.emoji_reactions.append(new_emoji) >+ new_emoji = "" >+ } >+ } >+ .font(.system(size: 14, weight: .bold)) >+ .frame(width: 80, height: 30) >+ .foregroundColor(.white) >+ .background(LINEAR_GRADIENT) >+ .clipShape(Capsule()) >+ .padding(EdgeInsets(top: 15, leading: 0, bottom: 0, trailing: 0)) >+ } >+ } >+ } >+ } >+ >+ Picker(NSLocalizedString("Select default emoji", comment: "Prompt selection of user's default emoji reaction"), >+ selection: $settings.default_emoji_reaction) { >+ ForEach(settings.emoji_reactions, id: \.self) { emoji in >+ Text(emoji) >+ } >+ } >+ >+ Section { >+ List(Array(settings.emoji_reactions), id: \.self) { emoji in isn't this already an Array? >+ EmojiView(settings: settings, emoji: emoji, recommended: false, showActionButtons: $showActionButtons) >+ } >+ } header: { >+ HStack { >+ Text(NSLocalizedString("Emoji Reactions", comment: "Section title for emoji reactions that are currently added.")) Text's are already localizable, you can just do: Text("Emoji Reactions", comment: ...) >+ .font(.system(size: 18, weight: .heavy)) >+ .padding(.bottom, 5) >+ } >+ } >+ >+ if recommended.count > 0 { >+ Section { >+ List(Array(recommended), id: \.self) { emoji in >+ EmojiView(settings: settings, emoji: emoji, recommended: true, showActionButtons: $showActionButtons) >+ } >+ } header: { >+ Text(NSLocalizedString("Recommended Emojis", comment: "Section title for recommend emojis")) Text("Recommended Emojis", comment: ...) >+ .font(.system(size: 18, weight: .heavy)) >+ .padding(.bottom, 5) >+ } >+ } >+ } >+ .navigationTitle(NSLocalizedString("Reactions", comment: "Title of emoji reactions view")) >+ .navigationBarTitleDisplayMode(.large) >+ .toolbar { >+ if showActionButtons { >+ Button("Done") { >+ showActionButtons.toggle() >+ } >+ } else { >+ Button("Edit") { >+ showActionButtons.toggle() >+ } >+ } >+ } >+ } >+ >+ func getMissingRecommendedEmojis(added: [String], recommended: [String] = default_emoji_reactions) -> [String] { needs a code comment, I don't know what this function is for >+ let addedSet = Set(added) >+ let missingEmojis = recommended.filter { !addedSet.contains($0) } >+ return missingEmojis >+ } >+} >+ >+struct AddEmojiView: View { >+ @Binding var emoji: String >+ >+ var body: some View { >+ ZStack(alignment: .leading) { >+ HStack{ >+ TextField(NSLocalizedString("⚡", comment: "Placeholder example for an emoji reaction"), text: $emoji) >+ .padding(2) >+ .padding(.leading, 25) >+ .opacity(emoji == "" ? 0.5 : 1) >+ .autocorrectionDisabled(true) >+ .textInputAutocapitalization(.never) >+ .onChange(of: emoji) { newEmoji in >+ if let lastEmoji = newEmoji.last.map(String.init), isValidEmoji(lastEmoji) { >+ self.emoji = lastEmoji >+ } else { >+ self.emoji = "" >+ } >+ } >+ >+ Label("", image: "close-circle") >+ .foregroundColor(.accentColor) >+ .padding(.trailing, -25.0) >+ .opacity((emoji == "") ? 0.0 : 1.0) >+ .onTapGesture { >+ self.emoji = "" >+ } >+ } >+ >+ Label("", image: "copy2") >+ .padding(.leading, -10) >+ .onTapGesture { >+ if let pastedEmoji = UIPasteboard.general.string { >+ self.emoji = pastedEmoji >+ } >+ } >+ } >+ } >+} >+ >+struct EmojiView: View { there are too many views in this file, let's make sure views like this are in their own file and have their own previews so that we can use the preview feature in xcode. >+ @ObservedObject var settings: UserSettingsStore >+ >+ let emoji: String >+ let recommended: Bool >+ >+ @Binding var showActionButtons: Bool >+ >+ var body: some View { >+ Group { >+ HStack { >+ if showActionButtons { >+ if recommended { >+ AddButton() >+ } else { >+ RemoveButton() >+ } >+ } >+ >+ Text(emoji) >+ } >+ } >+ .swipeActions { >+ if (!recommended){ nit: if !recommended { >+ RemoveButton() >+ .tint(.red) >+ } else { >+ AddButton() >+ .tint(.green) >+ } >+ } >+ .contextMenu { >+ CopyAction(emoji: emoji) >+ } >+ } >+ >+ func CopyAction(emoji: String) -> some View { >+ Button { >+ UIPasteboard.general.setValue(emoji, forPasteboardType: "public.plain-text") >+ } label: { >+ Label(NSLocalizedString("Copy", comment: "Button to copy an emoji reaction"), image: "copy2") >+ } >+ } >+ >+ func RemoveButton() -> some View { >+ Button(action: { >+ if let index = settings.emoji_reactions.firstIndex(of: emoji) { >+ settings.emoji_reactions.remove(at: index) >+ } would we ever not have an index here? >+ }) { >+ Image(systemName: "minus.circle") >+ .resizable() >+ .frame(width: 20, height: 20) >+ .foregroundColor(.red) >+ .padding(.leading, 5) >+ } >+ } >+ >+ func AddButton() -> some View { >+ Button(action: { >+ settings.emoji_reactions.append(emoji) >+ }) { >+ Image(systemName: "plus.circle") >+ .resizable() >+ .frame(width: 20, height: 20) >+ .foregroundColor(.green) >+ .padding(.leading, 5) >+ } >+ } >+} >+ >+/// From: https://stackoverflow.com/a/39425959 >+extension Character { Let's put this in its own file >+ /// A simple emoji is one scalar and presented to the user as an Emoji >+ var isSimpleEmoji: Bool { >+ guard let firstScalar = unicodeScalars.first else { return false } >+ return firstScalar.properties.isEmoji && firstScalar.value > 0x238C >+ } >+ >+ /// Checks if the scalars will be merged into an emoji >+ var isCombinedIntoEmoji: Bool { unicodeScalars.count > 1 && unicodeScalars.first?.properties.isEmoji ?? false } >+ >+ var isEmoji: Bool { isSimpleEmoji || isCombinedIntoEmoji } >+} >+ >+extension String { >+ var isSingleEmoji: Bool { count == 1 && containsEmoji } >+ >+ var containsEmoji: Bool { contains { $0.isEmoji } } >+ >+ var containsOnlyEmoji: Bool { !isEmpty && !contains { !$0.isEmoji } } >+ >+ var emojiString: String { emojis.map { String($0) }.reduce("", +) } >+ >+ var emojis: [Character] { filter { $0.isEmoji } } >+ >+ var emojiScalars: [UnicodeScalar] { filter { $0.isEmoji }.flatMap { $0.unicodeScalars } } >+} >+ >+func isValidEmoji(_ string: String) -> Bool { >+ return string.isSingleEmoji >+} >+ >+struct ReactionsSettingsView_Previews: PreviewProvider { >+ static var previews: some View { >+ ReactionsSettingsView(settings: UserSettingsStore()) >+ } >+} Getting there, thank you! jb