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.

Sign up for GitHub

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

Jump to bottom

Feature/sdl 0238 keyboard enhancements #474

Merged
merged 10 commits into from
Feb 12, 2021
Merged

Feature/sdl 0238 keyboard enhancements #474

merged 10 commits into from
Feb 12, 2021

Conversation

AKalinich-Luxoft
Copy link
Contributor

@AKalinich-Luxoft AKalinich-Luxoft commented Dec 18, 2020 •
edited by VjKlepikov
Loading

Implements #208

This PR is ready for review.

Testing Plan

Tested manually

Note: Manual_instruction_Keyboard_Enhancements_v1.2.zip

Summary

  • Add numeric keyboard implementation
  • Add keyboard controller with internal logic
  • Align keyboard layouts and style with proposal
  • Make proper processing of SetGlobalProperties
  • Set default app global properties
  • Add capabilities for new layouts and properties

CLA

@kboskin kboskin mentioned this pull request Jan 6, 2021
Merged
5 tasks
@AKalinich-Luxoft
Fix keyboard behavior according to proposal (#63)
d281e6e 
* Fix keyboard behavior according to proposal

1. `USER_CHOICE_INPUT_KEY_MASK` will change input masking to true by default (was false)
2. Send `OnKeyboardInput` notifications for all masking modes, but only after `SetGlobalProperties` and clicking mask/unmask button
@AKalinich-Luxoft
Copy link
Contributor Author

@ashwink11, this PR is ready for review

@ashwink11
Copy link

@AKalinich-Luxoft changes approved from the Ford side.

@KhrystynaDubovyk
Copy link

@JackLivio This PR is ready for Livio ready. Thank you!

@ashwink11
Copy link

@jordynmackool this PR is ready for review. Thanks!

jacobkeeler
jacobkeeler requested changes Jan 22, 2021
View reviewed changes
Copy link
Contributor

@jacobkeeler jacobkeeler left a comment

Choose a reason for hiding this comment

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

Note that this is a partial review due to the fact that there are some pending revisions which might affect the implementation of this slightly (see smartdevicelink/sdl_evolution#1117)

app/controller/sdl/KeyboardController.js Outdated Show resolved Hide resolved
app/controller/sdl/KeyboardController.js Outdated Show resolved Hide resolved
app/controller/sdl/KeyboardController.js Outdated Show resolved Hide resolved
app/controller/sdl/KeyboardController.js Outdated Show resolved Hide resolved
app/controller/sdl/KeyboardController.js Show resolved Hide resolved
@jordynmackool
Copy link

@AKalinich-Luxoft 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 @jacobkeeler to review when ready. Thanks!

@AKalinich-Luxoft
Copy link
Contributor Author

@jacobkeeler this PR has been updated according to revision and ready for Livio review

jacobkeeler
jacobkeeler approved these changes Feb 9, 2021
View reviewed changes
Copy link
Contributor

@jacobkeeler jacobkeeler left a comment •
edited
Loading

Choose a reason for hiding this comment

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

As a note, I did notice that the documentation for this feature says that the HMI should avoid duplicating custom keys, but this implementation does not do so. That said, I would see this as an optional optimization, so I am approving this PR.

@jacobkeeler jacobkeeler merged commit 2318bc9 into smartdevicelink:develop Feb 12, 2021
This was referenced Feb 12, 2021
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobkeeler jacobkeeler jacobkeeler approved these changes

@Jack-Byrne Jack-Byrne Awaiting requested review from Jack-Byrne

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@AKalinich-Luxoft @ashwink11 @KhrystynaDubovyk @jordynmackool @jacobkeeler