[#371] Add functionality for new keyboard expanded layout #373

damien-rivet · 2023-10-22T13:11:23Z

Contributor checklist

This pull request is on a separate branch and not the main branch

Description

This PR implements #371 by adding functionality for the new expanded layout that was introduced by #352.

The indent key is now fully functional as well as the capslock key.

Recordings

Old design

Simulator.Screen.Recording.-.iPad.Pro.12.9-inch.6th.generation.-.2023-10-22.at.14.39.31.-.01.mp4

New design

Simulator.Screen.Recording.-.iPad.Pro.12.9-inch.6th.generation.-.2023-10-22.at.14.44.49.-.01.-.01.mp4

Screenshots

Description	Screenshot
New design (iPad)
Design on iPhone (no changes)
Capslock
Uppercased suggestion

Related issue

Assign expanded iPad layouts for devices that are wide enough #352

github-actions · 2023-10-22T13:11:46Z

Thank you for the pull request!

The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and iOS rooms once you're in. It'd be great to have you!

Maintainer checklist

The commit messages for the remote branch should be checked to make sure the contributor's email is set up correctly so that they receive credit for their contribution
- The contributor's name and icon in remote commits should be the same as what appears in the PR
- If there's a mismatch, the contributor needs to make sure that the email they use for GitHub matches what they have for git config user.email in their local Scribe-iOS repo
The CHANGELOG has been updated with a description of the changes for the upcoming release (if necessary)

henrikth93 · 2023-10-25T19:19:54Z

Tested it and it looks good to me.

andrewtavis · 2023-10-30T00:00:57Z

Really appreciate the overall care/detail here and especially some a classic sentence for the example, @damien-rivet! 🦊😊 Going through it all now. Sorry that this took a bit. I needed to switch contexts for another project to do UI/UX design, and that always tends to slow down other work (although fun 🙃).

andrewtavis · 2023-10-30T00:01:56Z

Keyboards/KeyboardsBase/KeyAnimation.swift

@@ -319,7 +319,7 @@ func setPhoneKeyPopCharSize(char: String) {
      keyPopChar.font = .systemFont(ofSize: letterKeyWidth / 1.15)
      keyHoldPopChar.font = .systemFont(ofSize: letterKeyWidth / 1.15)
    }
-  } else if shiftButtonState == .shift || shiftButtonState == .caps {
+  } else if shiftButtonState == .shift || capsLockButtonState == .locked {


Very intuitive :)

andrewtavis · 2023-10-30T00:03:16Z

Keyboards/KeyboardsBase/KeyboardKeys.swift

-      || key == "return"
-      || key == "hideKeyboard"
-    {
+    } else if ["123", ".?123", "return", "hideKeyboard"].contains(key) {


Appreciate you fixing these formatting/logic errors! 🙏

andrewtavis · 2023-10-30T00:04:54Z

Keyboards/KeyboardsBase/KeyboardKeys.swift


+  /// Adjusts the style of the button based on different states


Minor nit that I'm noting here is that we'd like this to be a complete sentence with a period :) I can get it though!

andrewtavis · 2023-10-30T00:09:29Z

Keyboards/KeyboardsBase/KeyboardStyling.swift

@@ -40,7 +40,9 @@ var keysThatAreSlightlyLarger = [
  "chevron.right",
  "shift",
  "shift.fill",
+  "capslock",


Again the dedication here is amazing. Looking into and finding keysThatAreSlightlyLarger is great, but also has me worried about the state of some of the code that we're using so many random variables everywhere.

A basic ask would be if you had ideas for how we can improve the code base, it'd be great if you could open some issues. Anything that you've seen that might be some solid refactoring would be very welcome!

Good point 🤔 I'll do a quick sweep over the codebase and open a few issues for code quality ^^

Thanks so much, @damien-rivet! Always looking for some good first issues for new folks to work on 😊

andrewtavis · 2023-10-30T00:11:23Z

Keyboards/KeyboardsBase/KeyboardViewController.swift

@@ -406,7 +406,9 @@ class KeyboardViewController: UIInputViewController {
          var i = 0
          if completionOptions.count <= 3 {
            while i < completionOptions.count {
-              if shiftButtonState == .caps {
+              if shiftButtonState == .shift {


Interesting, we were missing shift?? Great catch :)

Yup, suggestions were not handled properly for capitalised first-letter but it will be history after the merge.

andrewtavis · 2023-10-30T00:12:30Z

Keyboards/KeyboardsBase/KeyboardViewController.swift

@@ -1925,6 +1937,10 @@ class KeyboardViewController: UIInputViewController {

          // Set the width of the key given device and the given key.
          btn.adjustKeyWidth()
+
+          // Update the button style


Noting the comment for myself.

andrewtavis · 2023-10-30T00:13:55Z

Keyboards/KeyboardsBase/KeyboardViewController.swift

@@ -2482,7 +2498,14 @@ class KeyboardViewController: UIInputViewController {
      }

    case "shift":
-      shiftButtonState = shiftButtonState == .normal ? .shift : .normal
+      if capsLockButtonState == .locked {
+        // Return capitalization to default


Nice as well as we now have both states to account for :)

andrewtavis · 2023-10-30T00:15:27Z

Keyboards/KeyboardsBase/KeyboardViewController.swift

@@ -2657,6 +2684,15 @@ class KeyboardViewController: UIInputViewController {
    }
  }

+  private func switchToFullCaps() {


Another thing that we've been dealing with is that not enough of the code base is split into functions. I do some rounds from time to time and break some things out, but if you came across some parts where we can extract some code then by all means let me know or open an issue!

andrewtavis · 2023-10-30T00:32:28Z

Keyboards/KeyboardsBase/KeyboardKeys.swift

+      if [.translate, .conjugate, .plural].contains(commandState) {
+        // Color the return key depending on if it's being used as enter for commands.
+        backgroundColor = commandKeyColor
+      }


We need the else back here for one bug I'm about to mention such that we get backgroundColor = specialKeyColor.

There's a default case which should handle the elsebut I'll do a review of my code to see if it's not missing some cases.

andrewtavis · 2023-10-30T00:33:34Z

Keyboards/KeyboardsBase/KeyboardKeys.swift

      if shiftButtonState == .shift {
        backgroundColor = keyPressedColor
        styleIconBtn(btn: self, color: UIColor.label, iconName: "shift.fill")
-      } else if shiftButtonState == .caps {
-        backgroundColor = keyPressedColor
-        styleIconBtn(btn: self, color: UIColor.label, iconName: "capslock.fill")


Losing this line is disabling an indication that we're in caps lock on iPhones :)

andrewtavis · 2023-10-30T00:35:52Z

Two things I'm seeing based on testing:

For iPhones we now have that the shift key doesn't change to the caps lock key when it's double tapped
- The caps lock functionality still works, but there's no change in the button to indicate that it will
- See this comment
The return key is changing to Scribe blue when we're in command mode, but it's white instead of grey for a special key when we're in the normal keyboard functionality (see screens)
- See this comment

Aside from this, really amazing work and one of our best first time contributions, @damien-rivet! I'm happy to get to the final fixes myself. Thank you!

damien-rivet · 2023-10-30T16:08:44Z

Two things I'm seeing based on testing:

For iPhones we now have that the shift key doesn't change to the caps lock key when it's double tapped

The caps lock functionality still works, but there's no change in the button to indicate that it will

See this comment

The return key is changing to Scribe blue when we're in command mode, but it's white instead of grey for a special key when we're in the normal keyboard functionality (see screens)

See this comment

Aside from this, really amazing work and one of our best first time contributions, @damien-rivet! I'm happy to get to the final fixes myself. Thank you!

Don't worry about the bugs, I'll be working on them asap, they should be pretty forward to iron out.

Fixed issue with CapsLock state not showing on iPhone. Added a missing period in a description. Fixed an issue with the Return key having the wrong color in some cases. Fixed a crash in Command mode when no input is supplied by the user.

damien-rivet · 2023-10-30T16:59:23Z

@andrewtavis I've fixed both issues in my last commit.

While testing I discovered a crash (nil unwrapping) in Command mode, I've added some checks to prevent those from happening again.

andrewtavis · 2023-10-30T17:44:12Z

Fantastic, @damien-rivet! I'm off to dinner, but will get to the final review this evening :) :)

andrewtavis

Thanks for getting to the changes so quickly, @damien-rivet, as well as finding and fixing the nil unwrapping! Really great to have this contribution :) Will bring it in and check to make sure that we have everything updated as far as the changelog and such, and then will close out the issue 😊

[scribe-org#371] Add functionality for new keyboard expanded layout

03fa49c

damien-rivet force-pushed the feature/371-add-functionality-new-keyboard-expendend-layouts branch from 5a4fac8 to 03fa49c Compare October 22, 2023 13:21

andrewtavis self-requested a review October 29, 2023 23:55

andrewtavis added the hacktoberfest-accepted Accepted as a part of Hacktoberfest label Oct 29, 2023

andrewtavis reviewed Oct 30, 2023

View reviewed changes

[scribe-org#371] Apply PR feedback

77f3b5c

Fixed issue with CapsLock state not showing on iPhone. Added a missing period in a description. Fixed an issue with the Return key having the wrong color in some cases. Fixed a crash in Command mode when no input is supplied by the user.

andrewtavis approved these changes Oct 31, 2023

View reviewed changes

andrewtavis merged commit 1edfd53 into scribe-org:main Oct 31, 2023

andrewtavis mentioned this pull request Oct 31, 2023

Add functionality to new keyboard keys on expanded layouts #371

Closed

2 tasks

damien-rivet deleted the feature/371-add-functionality-new-keyboard-expendend-layouts branch October 31, 2023 12:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#371] Add functionality for new keyboard expanded layout #373

[#371] Add functionality for new keyboard expanded layout #373

damien-rivet commented Oct 22, 2023 •

edited

Loading

github-actions bot commented Oct 22, 2023 •

edited by andrewtavis

Loading

henrikth93 commented Oct 25, 2023

andrewtavis commented Oct 30, 2023

andrewtavis Oct 30, 2023

andrewtavis Oct 30, 2023 •

edited

Loading

andrewtavis Oct 30, 2023

andrewtavis Oct 30, 2023

damien-rivet Oct 30, 2023

andrewtavis Oct 30, 2023

andrewtavis Oct 30, 2023

damien-rivet Oct 30, 2023

andrewtavis Oct 30, 2023

andrewtavis Oct 30, 2023

andrewtavis Oct 30, 2023

andrewtavis Oct 30, 2023

damien-rivet Oct 30, 2023

andrewtavis Oct 30, 2023

andrewtavis commented Oct 30, 2023

damien-rivet commented Oct 30, 2023

damien-rivet commented Oct 30, 2023

andrewtavis commented Oct 30, 2023

andrewtavis left a comment


		/// Adjusts the style of the button based on different states

[#371] Add functionality for new keyboard expanded layout #373

[#371] Add functionality for new keyboard expanded layout #373

Conversation

damien-rivet commented Oct 22, 2023 • edited Loading

Contributor checklist

Description

Recordings

Screenshots

Related issue

github-actions bot commented Oct 22, 2023 • edited by andrewtavis Loading

Thank you for the pull request!

Maintainer checklist

henrikth93 commented Oct 25, 2023

andrewtavis commented Oct 30, 2023

Choose a reason for hiding this comment

andrewtavis Oct 30, 2023 • edited Loading

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 30, 2023

damien-rivet commented Oct 30, 2023

damien-rivet commented Oct 30, 2023

andrewtavis commented Oct 30, 2023

andrewtavis left a comment

Choose a reason for hiding this comment

damien-rivet commented Oct 22, 2023 •

edited

Loading

github-actions bot commented Oct 22, 2023 •

edited by andrewtavis

Loading

andrewtavis Oct 30, 2023 •

edited

Loading