Closed Bug 830210 Opened 8 years ago Closed 7 years ago

[MUSIC]: not able to uncheck song rating in Music application

Categories

(Firefox OS Graveyard :: Gaia::Music, defect, P3)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:leo+, b2g18 fixed)

RESOLVED FIXED
1.1 QE1 (5may)
blocking-b2g leo+
Tracking Status
b2g18 --- fixed

People

(Reporter: leo.bugzilla.gaia, Assigned: leo.bugzilla.gaia)

Details

(Whiteboard: [TD-9572])

Attachments

(2 files, 4 obsolete files)

No description provided.
1. Title : Not able to uncheck song ratings in music app 
2. Precondition : video with duration more than one hour should available in phone
3. Tester's Action : User want to give a song zero or no rating 
4. Detailed Symptom (ENG.) : When user wants to give song zero rating or when he wanted to remove rating for a song, user will not able to uncheck (or remove) the rating for a song in Music app. 
5. Detailed Symptom (KOR.) : 
6. Expected : Music player should support to unchecking the rating for a song when user wants to clear the rating of that song.
7.Reproducibility: Y
1)Frequency Rate : 100%
8.Comparison Results : 
1)Model Comparing : 
9. Attached files: 
1)Log : 
2)Test Contents : 
3)Video file :
Summary: [MUSIC]: rating → [MUSIC]: not able to uncheck song rating in Music application
1. Title : Not able to uncheck song ratings in music app 
2. Precondition : Rating option should be available for all songs in music app.
3. Tester's Action : User want to give a song zero or no rating 
4. Detailed Symptom (ENG.) : When user wants to give song zero rating or when he wanted to remove rating for a song, user will not able to uncheck (or remove) the rating for a song in Music app. 
5. Detailed Symptom (KOR.) : 
6. Expected : Music player should support to unchecking the rating for a song when user wants to clear the rating of that song.
7.Reproducibility: Y
1)Frequency Rate : 100%
8.Comparison Results : 
1)Model Comparing : 
9. Attached files: 
1)Log : 
2)Test Contents : 
3)Video file :
Music app should be able to rate a song to 0 star... That's a bug, thanks!
Assignee: nobody → dkuo
blocking-b2g: --- → tef?
blocking-b2g: tef? → -
Please review the patch at pointer
https://github.com/mozilla-b2g/gaia/pull/8172
Attachment #715127 - Flags: review?(dkuo)
Assignee: dkuo → leo.bugzilla.gaia
I noticed the behaviour of rating songs might need to change if we apply this patch, and I would like to ask some inputs from UX.

What this patch did is unchecking the all stars when the we tap on the first star(if the first star is already checked), it also means that by tapping the first star, the user will feel it just like a toggle, it can be turned on/off.

But for the other four stars, there is no such behaviour like this, we better apply this to all the five stars, and I hope since we are going to fix this, we can also add this behaviour to complete this patch.

Casey, can you please comment this? thanks.
Flags: needinfo?(kyee)
(In reply to Dominic Kuo [:dkuo] from comment #5)
> What this patch did is unchecking the all stars when the we tap on the first
> star(if the first star is already checked), it also means that by tapping
> the first star, the user will feel it just like a toggle, it can be turned
> on/off.

Sounds like a good solution to me.

> 
> But for the other four stars, there is no such behaviour like this, we
> better apply this to all the five stars, and I hope since we are going to
> fix this, we can also add this behaviour to complete this patch.

I think adding the same toggle to all the stars also makes sense to me.
Flags: needinfo?(kyee)
CC Rob and Neo for additional suggestions if they have them.
blocking-b2g: - → leo?
leo.bugzilla.gaia@gmail.com, sorry for the late response, can you update your patch base on comment 5 and comment 6, which are comfirmed by our ux Casey? thanks.
Comment on attachment 715127 [details]
Pointer to pull request - https://github.com/mozilla-b2g/gaia/pull/8172

leo.bugzilla.gaia@gmail.com, I am cancelling this review request first to notify you, after this patch is updated, please re-assign to me again, thanks.
Attachment #715127 - Flags: review?(dkuo)
triage: leo+ as user cannot make any further changes to rating. seem like a broken features
blocking-b2g: leo? → leo+
Attached file Music rating toggle and check/Uncheck (obsolete) —
As per the comments on github, uploaded the patch.
1. Patch also fixes the first star check and uncheck
2. Patch toggles all individual ratings
Attachment #735706 - Flags: review?(dkuo)
Attachment #715127 - Attachment is obsolete: true
added patch for Music rating along with toggling of each rate
Attachment #735808 - Flags: review?(dkuo)
obsolete
Attachment #735706 - Attachment is obsolete: true
Attachment #735808 - Attachment is obsolete: true
Attachment #735706 - Flags: review?(dkuo)
Attachment #735808 - Flags: review?(dkuo)
Attachment #735814 - Flags: review?(dkuo)
Whiteboard: [TD-9572]
Comment on attachment 735814 [details]
patch for Music rating along with toggling of each rate

leo.bugzilla.gaia@gmail.com The patch looks okay but need to revise, please see my comments on github.

I am cancelling this review request first to notify you, after this patch is updated, please re-assign to me again, thanks.
Attachment #735814 - Flags: review?(dkuo)
Whiteboard: [TD-9572] → [TD-9572] [need revised patch]
Hi dkuo,

Refer my comments in github and please suggest .

Thanks,
Flags: needinfo?(dkuo)
leo.bugzilla.gaia@gmail.com, I have commented my suggestion on github, thanks.
Flags: needinfo?(dkuo)
Target Milestone: --- → Leo QE1 (5may)
Attachment #735814 - Attachment is obsolete: true
Attachment #738916 - Flags: review?(dkuo)
Priority: -- → P3
Comment on attachment 738916 [details]
Patch for music rating, includes toggling of individual rate icon

The patch looks good to me now. Leo, please see the comments on github and fix the issues I have mentioned before landing it, thanks.
Attachment #738916 - Flags: review?(dkuo) → review+
Hi dkuo,

Fixed as per your comments on github.
patch uploaded, Setting need a info to intimate you .

Thanks,
Flags: needinfo?(dkuo)
Leo, thanks for updating your patch and you should also rebase you branch then I can merge for you.
Flags: needinfo?(dkuo)
Hi Dkuo,

Currently the patch what i have uploaded is on GAIA Master's, Can you please tell me to which branch i should rebase.

Thanks,
Flags: needinfo?(dkuo)
Your PR is from leob2g:Bug_830210_Music_rating_fix and it's out-of-date, so you can just rebase that branch, like pull the latest upstream then rebase your local branch, and push back to leob2g:Bug_830210_Music_rating_fix, after that your PR should be automatically updated.
Flags: needinfo?(dkuo)
Hi dkuo,

I have rebased, as per your comment.
Please confirm .

Thanks
Flags: needinfo?(dkuo)
Comment on attachment 738916 [details]
Patch for music rating, includes toggling of individual rate icon

><!DOCTYPE html><meta charset="utf-8"><meta http-equiv="refresh" content="5;https://github.com/mozilla-b2g/gaia/pull/9480"><title>Bugzilla Code Review</title><p>You can review this patch at <a href="https://github.com/mozilla-b2g/gaia/pull/9480">https://github.com/mozilla-b2g/gaia/pull/9480</a>, or wait 5 seconds to be redirected there automatically.</p>
Flags: needinfo?(dkuo)
Leo, the commit message of attachment 738916 [details] didn't follow the gaia policy, so I cherry picked your commit, refine it and opened a new PR, in attachment 743527 [details].
Landed on master: de39459d28922fe4e6e8385eddf3104d124a72d6
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [TD-9572] [need revised patch] → [TD-9572]
Uplifted de39459d28922fe4e6e8385eddf3104d124a72d6 to:
v1-train: 874217478cd16eb7430fc73cd442735b7c3b726e
Flags: in-moztrap?
Added test case in MozTrap: 
https://moztrap.mozilla.org/manage/case/8518/
Flags: in-moztrap? → in-moztrap+
You need to log in before you can comment on or make changes to this bug.