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 0198 - Media Skip Indicators #596

Closed
theresalech opened this issue Aug 29, 2018 · 8 comments
Closed

[Accepted with Revisions] SDL 0198 - Media Skip Indicators #596

theresalech opened this issue Aug 29, 2018 · 8 comments

Comments

@theresalech
Copy link
Contributor

Hello SDL community,

The review of "SDL 0198 - Media Skip Indicators" begins now and runs through September 4, 2018. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0198-media-skip-indicators.md

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

#596

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
theresa@livio.io

@robinmk
Copy link
Contributor

robinmk commented Aug 31, 2018

I support the idea but have a few comments/questions:

  1. Should the seekTime parameter be limited to 99 seconds?

  2. Probably a typo but shouldn't the type for forwardSeekIndicator and backSeekIndicator be SeekStreamingIndicator?

  3. By default the seek indicators should be TRACK when: the media app was closed by the user (App enters HMI_NONE)

I think this is not required and the system can remember that the seek buttons should be of type TIME since this indicator seems more like a property (maybe we should consider it to be part of SetGlobalProperties instead) rather than a state (which is the likely the reason for the design of SDL-0109)

  1. Minor issue with the formatting of the seekTime parameter.

@joeljfischer
Copy link
Contributor

Hi @robinmk

  1. I did that to help HMI designers to not have to account for 3 digits in their skip button designs, and because I haven't seen any podcast or audiobook apps with options for greater than 99 seconds (or even 60 seconds). We could raise it to 999 if desired, but I think that would be more difficult for UI designers.

  2. Yep

  3. I would like to keep the design consistent with SDL-0109. I definitely understand your point and thought about it, but I think keeping things predictable and consistent is more important in this case. If anyone else has thoughts, I'd love to hear them.

  4. Yes, it's missing an =.

@robinmk
Copy link
Contributor

robinmk commented Sep 5, 2018

  1. I guess it is not a big concern as it would be HMI design decision. However, app/proxy developers must be made aware to send the SeekIndicatorType whenever the app moves out of NONE.

Agree to other comments.

@jordynmackool
Copy link
Contributor

jordynmackool commented Sep 5, 2018

The Steering Committee voted on 2018-09-04, to defer this proposal, keeping it in
review until our next meeting on 2018-09-18, to allow the author to respond to
the questions on the review issue.

@MazdaMKok
Copy link

@jordynmackool What exactly were the other questions that the author had to respond to, because it seems that @robinmk had his questions answered? robinmk did note that app/proxy developers must be made aware to send the SeekIndicatorType whenever the app moves out of NONE, but besides that, I don't see any other questions to be answered. Maybe there was something I missed....

@jordynmackool jordynmackool changed the title [In Review] SDL 0198 - Media Skip Indicators [Accepted with Revisions] SDL 0198 - Media Skip Indicators Sep 19, 2018
@jordynmackool jordynmackool reopened this Sep 19, 2018
@jordynmackool
Copy link
Contributor

The Steering Committee voted to accept this proposal with revisions on 2018-09-18. The revisions will incorporate the feedback agreed to in this comment by the author, and will also make sure that conditions of SeekIndicatorType are documented, as requested in this comment.

@smartdevicelink smartdevicelink locked and limited conversation to collaborators Sep 19, 2018
@jordynmackool
Copy link
Contributor

jordynmackool commented Sep 20, 2018

Proposal has been revised, and issues have been entered:
Core
iOS
Android
Generic_hmi
Sdl_hmi
Rpc_spec

@theresalech
Copy link
Contributor Author

JavaScript Suite issue: smartdevicelink/sdl_javascript_suite#346

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

5 participants