#229 #96 adds french azerty and reduces JSON sizes #236

andrewtavis · 2022-10-17T21:30:34Z

What's in here:

All JSON files have had their indentation removed to reduce their filesizes (no need to check)
Changelog updates
Renames the French keyboard French AZERTY and adds French QWERTY
- I made individual interface files that define the keys for each, which is then referenced within a common interface file that defines things like command button titles, alternate keys for long presses, etc (see setFRKeyboardLayout)
- References to French are fixed throughout the project
Fixes the ´ character on French keyboards
- Also includes making sure it's sized correctly and doesn't shift with the letter keys

wkyoshida · 2022-10-17T22:14:03Z

🙌 will take a look at this!

andrewtavis · 2022-10-17T22:17:56Z

Ah I forgot that you need to be a collaborator for me to request a review :) Let me know if that's of interest and we can definitely make that happen after a couple of code contributions 😊 Thanks for taking a look!

wkyoshida

I didn't go over the JSON files 👀 but the French keyboard changes are looking good! Couple minor comments that I had ⬇️

wkyoshida · 2022-10-18T03:07:23Z

Keyboards/KeyboardsBase/ScribeFunctionality/Conjugate.swift

@@ -236,7 +239,7 @@ func returnConjugation(keyPressed: UIButton, requestedForm: String) {
 /// Returns the conjugation state to its initial conjugation based on the keyboard language.
 func resetVerbConjugationState() {
  conjViewShiftButtonsState = .leftInactive
-  if controllerLanguage == "French" {
+  if ["French_AZERTY", "French_QWERTY"].contains(controllerLanguage) {


Could it make sense to initialize ["French_AZERTY", "French_QWERTY"] globally perhaps? I'm thinking that in doing so, if a new layout variant is added, one wouldn't have to remember to make edits to multiple locations. Could then just update in one location.

Referring to the other changes made on L295 and L314 in this same file, for instance.

Makes sense :) I think the easiest way of doing this right now that would allow for variants to be added without needing to change them is to just pattern match on the string in the following way:

controllerLanguage.prefix("French".count) == "French"

Love it 👍 Yep, I agree! I think that'd be a pretty clean way of doing it if the variants all begin with the target language descriptor

Will have to, but I think it makes sense to do it that way anyway 😊

CHANGELOG.md

Keyboards/LanguageKeyboards/French/French-AZERTY/FR-AZERTYInterfaceVariables.swift

wkyoshida · 2022-10-18T04:12:52Z

Keyboards/LanguageKeyboards/French/FRInterfaceVariables.swift

 func setFRKeyboardLayout() {
-  getFRKeys()


Oh I was thinking that getFRKeys() might be able to stay in this file, maybe no? Could it work to change getFRKeys() to take in a parameter for FrenchKeyboardConstants and then the if statements would just call it with either the AZERTY or QWERTY constants sent in? Perhaps you did try that already and there's just something I'm overlooking

I made the decision to not include this with an eye on adding future ones that might have more distinct variations in the future, but thinking about it now I can't think of any that would be sooo different that we wouldn't be able to handle their variations in conditions at the top as we do with setFRKeyboardLayout. Thanks for bringing this up 😊

But now I'm realizing that there actually is an issue here, as we'd need to make a var that can be assigned either of the constant enums, but then what happens is that this var would need to be instantiated with all the elements of the enums so that Swift/Xcode will know that they exist. I'm now getting errors that say that FrenchKeyboardConstants.letterKeysPhone doesn't exist. We should likely create a base enum that all the other interface variable ones are based on :) Would be a different issue though 😊

Ahh I see.. 😞 I was concerned something like that might get in the way.

Yep, I agree though. This can get refined later on in other issues, especially as work gets done on decoupling the language and the keyboard layout.

wkyoshida · 2022-10-18T04:31:27Z

Keyboards/KeyboardsBase/KeyboardKeys.swift

@@ -51,7 +51,11 @@ class KeyboardKey: UIButton {
      self.layer.setValue(true, forKey: "isSpecial")


Just an observation on the changes in this file -

These are interesting, hmm.. 🤔 Makes sense that some keys should be ignored when it comes to upper/lower-casing. Just wondering if there could be a scenario where a key would have upper/lower-casing in one language but not in another. In that case, the logic here would depend on the target language. I'm thinking this would definitely be an edge-case, and I don't even know if it could happen 😆 , but just a thought I had.

Thoughts of edge cases are much appreciated! There are all kinds of little things like that in Scribe 😊 Specifically languagesWithCapitalizedNouns, which as of now is ["German"] 😄 We try to handle them as they come up :)

Oh.. the challenges of working with multiple human languages 😆

Welcome to Scribe! 🤣😊

Co-authored-by: wkyoshida <yoshida.william@gmail.com>

…rfaceVariables.swift Co-authored-by: wkyoshida <yoshida.william@gmail.com>

…m/scribe-org/Scribe-iOS into 229-96-french-azerty-shrink-jsons

andrewtavis · 2022-10-18T07:00:26Z

Thanks for the review, @wkyoshida! I'll merge this now and we can discuss a bit more in the issue 🚀

andrewtavis added 2 commits October 17, 2022 23:28

#229 #96 adds french azerty and reduces JSON sizes

22dc867

#229 fix French view controller class name

a953e8a

andrewtavis mentioned this pull request Oct 17, 2022

Regional keyboard variants #229

Closed

2 tasks

#229 fix French currency symbol and prevent lowercase ABC

9b82ebd

andrewtavis assigned wkyoshida and andrewtavis Oct 17, 2022

andrewtavis linked an issue Oct 17, 2022 that may be closed by this pull request

Regional keyboard variants #229

Closed

2 tasks

wkyoshida reviewed Oct 18, 2022

View reviewed changes

andrewtavis and others added 4 commits October 18, 2022 08:32

Update CHANGELOG.md

cebaa3e

Co-authored-by: wkyoshida <yoshida.william@gmail.com>

Update Keyboards/LanguageKeyboards/French/French-AZERTY/FR-AZERTYInte…

216940d

…rfaceVariables.swift Co-authored-by: wkyoshida <yoshida.william@gmail.com>

#229 simplify French conjugation function checks

343b1f9

Merge branch '229-96-french-azerty-shrink-jsons' of https://github.co…

6412302

…m/scribe-org/Scribe-iOS into 229-96-french-azerty-shrink-jsons

andrewtavis merged commit 3fbd32a into main Oct 18, 2022

andrewtavis deleted the 229-96-french-azerty-shrink-jsons branch October 18, 2022 07:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#229 #96 adds french azerty and reduces JSON sizes #236

#229 #96 adds french azerty and reduces JSON sizes #236

andrewtavis commented Oct 17, 2022 •

edited

Loading

wkyoshida commented Oct 17, 2022

andrewtavis commented Oct 17, 2022 •

edited

Loading

wkyoshida left a comment

wkyoshida Oct 18, 2022

andrewtavis Oct 18, 2022

wkyoshida Oct 19, 2022

andrewtavis Oct 19, 2022

wkyoshida Oct 18, 2022

andrewtavis Oct 18, 2022

andrewtavis Oct 18, 2022

wkyoshida Oct 19, 2022

wkyoshida Oct 18, 2022

andrewtavis Oct 18, 2022

wkyoshida Oct 19, 2022

andrewtavis Oct 19, 2022

andrewtavis commented Oct 18, 2022

		@@ -51,7 +51,11 @@ class KeyboardKey: UIButton {
		self.layer.setValue(true, forKey: "isSpecial")

#229 #96 adds french azerty and reduces JSON sizes #236

#229 #96 adds french azerty and reduces JSON sizes #236

Conversation

andrewtavis commented Oct 17, 2022 • edited Loading

wkyoshida commented Oct 17, 2022

andrewtavis commented Oct 17, 2022 • edited Loading

wkyoshida left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewtavis commented Oct 18, 2022

andrewtavis commented Oct 17, 2022 •

edited

Loading

andrewtavis commented Oct 17, 2022 •

edited

Loading