Implement SDL-0216 Widget Support #1350
Conversation
Add RPC Name for Create/Delete Window
Add Files for Window Type
Change the setters and getters on the Create window request
Add template to the parameter names
add template configuration to the parameter names
…ities, presetBankCapabilities from RegisterAppInterfaceResponse
RPC Names
mjuarez-ford
changed the title
Co-Authored-By: Joel Fischer <joeljfischer@gmail.com>
…m/mjuarez-ford/sdl_ios into feature/issue_0216_widget_support
|
Thanks @joeljfischer I made the suggested changes and fixes. I just have one commet regarding the |
|
@joeljfischer the int and enum changes are done. The additional init shouldn’t be key for this proposal. Due to timing I would suggest to review and merge if no issue is left. I would like to start to work on the subsequent proposal and the capability converter #1386 |
|
Thanks @joeljfischer @kshala-ford I made the suggested changes. |
| @@ -17,6 +17,7 @@ | |||
|
|
|||
| NS_ASSUME_NONNULL_BEGIN | |||
|
|
|||
| __deprecated_msg("This RPC is deprecated. Use Show RPC to change layout when connected to a 6.0+ head unit. Since sdl_ios v6.4 / RPC spec 6.0") | |||
There was a problem hiding this comment.
We've been discussing this a lot internally after tests with example apps have shown that this is a very confusing deprecation. Developers have to know that they must use this RPC when connected to systems < 6.0, and can use this method when connected to systems > 6.0 but not when changing widget layouts, where they must use Show.
It's obviously pretty confusing that developers must use a deprecated RPC to set layouts in most situations, but must not use it in other situations. Even worse is that despite that they must use the deprecated RPC, the compiler is going to throw a warning every time they do.
The conclusion that we came to is that even though the RPC is deprecated, we shouldn't deprecate this class in this PR and release. That way developers won't be faced with confusing deprecation warnings that they shouldn't actually do anything about. Second, we'll have to create very good documentation regarding changing widget layouts.
This isn't ideal and we can't alter the proposal at this point (and thus the RPC spec itself), but I think we'll have to do this in the iOS library (which isn't going against the letter of the proposal) in order to not confuse developers. We can also remove many of the #pragmas around SetDisplayLayout deprecations in the library at that point.
There was a problem hiding this comment.
We've been discussing this a lot internally after tests with example apps have shown that this is a very confusing deprecation. Developers have to know that they must use this RPC when connected to systems < 6.0, and can use this method when connected to systems > 6.0 but not when changing widget layouts, where they must use Show.
I want to note the case that widgets are introduced with 6.0 hence logically the highlighted text isn't necessary to cover. So the concern is
version<6.0 use SetDisplayLayout && version>=6.0 use Show
I understand the concern. I believe this issue should be solved by introducing template configuration to the managers. Similar to what "Manager Update for Display Capability" does we should work on a proposal like "Manager Update for Template Configuration". If the screen manager would handle template configuration we could keep SetDisplayLayout deprecated. With this feature the deprecation message could be to suggest using ScreenManager and for Widgets using Show (as long as there are no widget manager... work on it...).
Without the manager update the deprecation is confusing. Here's what I would suggest:
- The deprecation for
SetDisplayLayoutremains in the mobile API. - In iOS and Java
SetDisplayLayoutshould not be marked as deprecated (we remove the deprecation flag) - We work on a new proposal like "Manager Update for Template Configuration" which can be separate or part of a proposal like "Widget Screen Manager"
- The proposal doesn't modify mobile API but only the libraries (therefore can be implemented into the library without a Core change)
There was a problem hiding this comment.
@joeljfischer @kshala-ford The changes has been applied for iOS.
But for Android the merge request has been already been approved, so how should we proceed.
There was a problem hiding this comment.
The Android team here will take care of making the required changes to match @theresalech
There was a problem hiding this comment.
I'm still doing final testing with Core which I should finish on Tuesday, but I have one more requested change leading up to that: there are numerous warnings throughout the tests as a result of the newly deprecated properties. Please update the tests with #pragma to disable the warnings.
Co-Authored-By: Kujtim Shala <kshala@ford.com>
|
@joeljfischer the ignore of deprecated properties on the tests is complete. |
Fixes #1270
This PR is ready for review.
Risk
This PR makes minor API changes.
Testing Plan
Unit test for new classes and properties.
Unit test and smoke test for the screen manager. (display capability redesign).
Summary
Implement proposal as defined
Tasks Remaining:
CLA