Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDL-0238 Keyboard Enhancements #1883

Conversation

yoooriii
Copy link
Contributor

@yoooriii yoooriii commented Dec 18, 2020

Fixes #1356

This PR is ready for review.

Risk

This PR makes minor API changes.

Testing Plan

  • I have verified that I have not introduced new warnings in this PR (or explain why below)
  • I have run the unit tests with this PR
  • I have tested this PR against Core and verified behavior (if applicable, if not applicable, explain why below).

Unit Tests

New tests:
SDLKeyboardInputMaskSpec.m
SDLKeyboardCapabilitiesSpec.m
SDLKeyboardLayoutCapabilitySpec.m
Updated tests:
SDLKeyboardEventSpec.m
SDLKeyboardLayoutSpec.m
SDLKeyboardPropertiesSpec.m
SDLWindowCapabilitySpec.m

Core Tests

List of tests performed against Core and behaviours verified
Core version / branch / commit hash / module tested against: sdl_0238_keyboard_enhancements
HMI name / version / branch / commit hash / module tested against: sdl_0238_keyboard_enhancements

Summary

Added a new keyboard layout and added new properties for keyboards supported by SDL.

  1. Support for Numeric Keyboard.
  2. Allow apps to mask entered characters.
  3. Allow apps to change special characters shown on the keyboard layout.

Changelog

Breaking Changes
  • no breaking changes
Enhancements
  • no enhancements
Bug Fixes
  • no bug fixes

Tasks Remaining:

  • none

CLA

@codecov
Copy link

codecov bot commented Dec 18, 2020

Codecov Report

Merging #1883 (b2d18db) into develop (74852b5) will increase coverage by 0.06%.
The diff coverage is 94.27%.

@@             Coverage Diff             @@
##           develop    #1883      +/-   ##
===========================================
+ Coverage    84.94%   85.00%   +0.06%     
===========================================
  Files          426      428       +2     
  Lines        21383    21559     +176     
===========================================
+ Hits         18164    18327     +163     
- Misses        3219     3232      +13     

@a-prykho
Copy link

@standa1-ford this PR is ready for Ford review

…iew) and update -init method to make Codecov happy.
@yoooriii
Copy link
Contributor Author

yoooriii commented Jan 6, 2021

Hello @standa1-ford, I have fixed your comment, could you please accept it? Thank you.

@yoooriii yoooriii marked this pull request as ready for review January 6, 2021 11:10
Copy link
Contributor

@standa1-ford standa1-ford left a comment

Choose a reason for hiding this comment

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

Looks good!

@a-prykho
Copy link

a-prykho commented Jan 8, 2021

Dear @jordynmackool, @joeljfischer this PR is ready for Livio review

@jordynmackool
Copy link
Contributor

@standa1-ford is looks like changes were made to the PR after your approval - please confirm this is ready for Livio review.

@standa1-ford
Copy link
Contributor

@jordynmackool approved

@joeljfischer joeljfischer added manager-screen Relating to the manager layer - screen managers proposal Accepted SDL Evolution Proposal rpc Relating to the RPC layer labels Jan 25, 2021
@jordynmackool
Copy link
Contributor

@yoooriii the proposal markdown file has been updated per the revisions included in the accepted with revisions review issue: Revise SDL-0238 Keyboard Enhancements.

Please make the needed updates to this PR and then tag @FrankElias77 to review when ready. Thanks!

@yoooriii
Copy link
Contributor Author

Hello @joeljfischer. I have fixed the comments, could you review please? Thank you.

@yoooriii
Copy link
Contributor Author

Note: for some reason SDLVoiceCommandManagerSpec.m fails at random on different tests, it has nothing to do with the PR.

@yoooriii
Copy link
Contributor Author

Hello @joeljfischer. I have implemented the requested logic. Could you please review? Thank you.

…thod -createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFromConfiguration:
@yoooriii
Copy link
Contributor Author

Hello @joeljfischer. I have implemented the requested changes. Could you please review the PR? Thank you.

Note: for some reason a unit test failed in SDLVoiceCommandManagerSpec on the previous run. It has nothing to do with the PR.

Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

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

A few more minor items

… couple of additional tests to cover custom keys trim logic
@yoooriii
Copy link
Contributor Author

Hello @joeljfischer. I have implemented the changes from the code review. Could you please proceed? Thank you.

Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

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

One missed item from last time

@yoooriii
Copy link
Contributor Author

One missed item from last time

@joeljfischer I am sorry, I missed this one. I have commited, Could you review please? Thank you.

Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

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

Last minute testing & alignment caught one more issue.

…g to SDLPresentChoiceSetOperation and add a unit test
@yoooriii
Copy link
Contributor Author

Hello @joeljfischer. I have implemented the requested logic and a unit test for it. Could you please take a look? Thank you.

@joeljfischer joeljfischer merged commit c9db35d into smartdevicelink:develop Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
manager-screen Relating to the manager layer - screen managers proposal Accepted SDL Evolution Proposal rpc Relating to the RPC layer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants