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

Feature/choice set manager #371

Merged
merged 20 commits into from
Feb 2, 2021
Merged

Feature/choice set manager #371

merged 20 commits into from
Feb 2, 2021

Conversation

crokita
Copy link
Contributor

@crokita crokita commented Jan 12, 2021

Partially fixes #123

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

Unit tests added that match the tests added in the Java Suite library for the choice set related classes

Summary

Adds the choice set manager which feature matches the Java Suite library for all the screen manager APIs related to the choice set manager.

Note that presentSearchableChoiceSet would not be supported in the latest develop Generic HMI, and that OnKeyboardInput messages haven't been received.

Copy link
Contributor

@renonick87 renonick87 left a comment

Choose a reason for hiding this comment

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

See requested feedback and remember to run npm run lint to resolve any issues in documentation.

@crokita
Copy link
Contributor Author

crokita commented Jan 25, 2021

PR updated. I didn't include unrelated linter fixes in this PR: that can be done in a separate PR. Also note:

  • I moved systemcapabilitylistener logic to the _ChoiceSetManagerBase to fix an issue with _displayName never being set by looking at the display capabilities
  • _ChoiceSetSelectionListener and _KeyboardListener are now public classes
  • Fixed ChoiceCell's equals() method to not error out when comparing null voice commands

@@ -32,10 +40,19 @@ describe('ManagerTests', function () {
voiceCommandUpdateOperationTests(appClient);
Copy link
Contributor

Choose a reason for hiding this comment

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

Something with the most recent update is causing failures in the voiceCommandManagerTests and voiceCommandUpdateOperationTests for me. I've tested this with manticore across multiple browsers. Everything passes if these tests are moved above the textAndGraphicManagerTests and run first though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's annoying. I have moved those tests and got the same results now.

@crokita crokita mentioned this pull request Jan 27, 2021
3 tasks
@crokita
Copy link
Contributor Author

crokita commented Jan 27, 2021

Updated. See comments in response to feedback

Copy link
Contributor

@renonick87 renonick87 left a comment

Choose a reason for hiding this comment

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

Since this PR contains updates from #357, merging the ChoiceSetManager will have to wait until after that PR is merged first.

@renonick87
Copy link
Contributor

renonick87 commented Feb 1, 2021

Updates from #357 have been reverted from this PR.

@crokita
Copy link
Contributor Author

crokita commented Feb 2, 2021

Squashing to avoid accidentally clearing out this PR: #357

@crokita crokita merged commit dd37461 into develop Feb 2, 2021
@jordynmackool jordynmackool added proposal Accepted SDL Evolution Proposal and removed proposal Accepted SDL Evolution Proposal labels Mar 8, 2021
@crokita crokita deleted the feature/choice-set-manager branch March 26, 2021 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SDL 0272] JavaScript Suite SDL Manager
4 participants