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

[Accepted with Revisions] SDL 0284 - DialNumber Text #950

Closed
theresalech opened this issue Feb 26, 2020 · 7 comments
Closed

[Accepted with Revisions] SDL 0284 - DialNumber Text #950

theresalech opened this issue Feb 26, 2020 · 7 comments

Comments

@theresalech
Copy link
Contributor

Hello SDL community,

The review of "SDL 0284 - DialNumber Text" begins now and runs through March 3, 2020. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0284-DialNumber_Text.md

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

#950

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

  • Is the problem being addressed significant enough to warrant a change to SDL?
  • Does this proposal fit well with the feel and direction of SDL?
  • If you have used competitors with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
    Please state explicitly whether you believe that the proposal should be accepted into SDL.

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you,
Theresa Lech

Program Manager - Livio
[email protected]

@joeygrover
Copy link
Member

1. Minor item, JavaScript Suite needs to be added to affected platforms since there is an RPC change.
2. The proposal only includes the HMI_API changes so the RPC Spec ones need to be added:

    <function name="DialNumber" functionID="DialNumberID" messagetype="request" since="3.0">
    .
    .
    .
        <param name="messageText" type="String" mandatory="false" since="X.X">
            <description>
              Body of text to display to the user.
            </description>
        </param>
    </function>

3. I'd also prefer naming the param to messagePrompt or similar. Open to suggestions and not a huge deal either way.

4. Should we define potential lengths to this text? I imagine it would be useful for developers and OEMs to know exactly what max length should be displayed in these prompts.

@joeljfischer
Copy link
Contributor

3. I think promptText might work? Something descriptive so that developers know how it will be used.

4. I think that adding a maxLength of 500 should probably work.

@Jack-Byrne
Copy link
Contributor

Jack-Byrne commented Feb 28, 2020

  1. @joeygrover Proxy can use the TextField capability struct to get the supported width / rows of the HMI's message prompt.

@joeljfischer
Copy link
Contributor

  1. You would have to add a new TextFieldName to the RPC enum here: https://github.com/smartdevicelink/rpc_spec/blob/master/MOBILE_API.xml#L753

I would recommend the TextFieldName value of dialNumberPromptText.

@theresalech theresalech changed the title [In Review] SDL 0284 - DialNumber Text [Accepted with Revisions] SDL 0284 - DialNumber Text Mar 4, 2020
@theresalech
Copy link
Contributor Author

The Steering Committee voted to accept this proposal with the following revisions:

  • Add JavaScript Suite as an impacted platform
  • Add RPC Spec changes
    <function name="DialNumber" functionID="DialNumberID" messagetype="request" since="3.0">
    .
    .
    .
        <param name="messageText" type="String" mandatory="false" since="X.X">
            <description>
              Body of text to display to the user.
            </description>
        </param>
    </function>
  • Use promptText as a parameter name in DialNumber, instead of messageText
  • Add dialNumberPromptText as a new TextFieldName

@theresalech
Copy link
Contributor Author

@MichaelCrimando please advise when a new PR has been entered to update the proposal to reflect the agreed upon revisions. I'll then merge the PR so the proposal is up to date, and enter issues in respective repositories for implementation. Thanks!

@smartdevicelink smartdevicelink locked as resolved and limited conversation to collaborators Mar 4, 2020
@theresalech
Copy link
Contributor Author

The author has updated the proposal to reflect the agreed upon revisions and implementation issues have been entered:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants