Closed jaydeepsingh25 closed 2 years ago
The PR fixes the Play/Pause issues raised in the previous PR. The new UI can be tested with the extension extension.zip. @jeffbl / @Sabrina-Knappe please let me know if these changes are good to merge now.
Observations:
@Cybernide suggest that you test as well before this goes into production before NFB, as it might impact demos / require video updates?
@jaydeepsingh25 Discussed with @Sabrina-Knappe and @Cybernide at tech-arch today, and this should NOT be merged, since they want to test it with panel before it goes live. Note that I have moved #168 to next sprint due to this.
Assigning to @Sabrina-Knappe to re-activate for merge when testing complete.
@jaydeepsingh25 There is still one problem with the logic. When a button is pressed, it should start the respective audio, but if the same button is pressed right after, it should pause that audio clip. When a different button is pressed, that audio should play and returning to a previous button should restart that audio.
The current behaviour has multiple sequential presses of the same button restarting the audio file every time.
Let me know if anything about this is unclear.
Assigning to @jaydeepsingh25 for logic fixes.
@jaydeepsingh25 I believe this is now blocking testing this with users. Is this something that can be done soon?
@jeffbl @jaydeepsingh25 Following up on this.
I have fixed the above mentioned problem, Now when the same button is pressed, it pauses the respective audio rather than playing it again. @jeffbl / @Sabrina-Knappe can you please do a quick round of testing to check if this behavior is in-line with the expectation. Please let me know if any further changes are required. If not, i guess we can proceed with user testing. Extension to be used for testing: extension.zip
Will do, thank you!
From: Jaydeep Singh @.> Date: Sunday, August 21, 2022 at 2:11 AM To: Shared-Reality-Lab/IMAGE-browser @.> Cc: Sabrina Knappe @.>, Mention @.> Subject: Re: [Shared-Reality-Lab/IMAGE-browser] remove dropdown (PR #233)
I have fixed the above mentioned problem, Now when the same button is pressed, it pauses the respective audio rather than playing it again. @jeffblhttps://github.com/jeffbl / @Sabrina-Knappehttps://github.com/Sabrina-Knappe can you please do a quick round of testing to check if this behavior is in-line with the expectation. Please let me know if any further changes are required. If not, i guess we can proceed with user testing. Extension to be used for testing: extension.ziphttps://github.com/Shared-Reality-Lab/IMAGE-browser/files/9388552/extension.zip
— Reply to this email directly, view it on GitHubhttps://github.com/Shared-Reality-Lab/IMAGE-browser/pull/233#issuecomment-1221476326, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ANL6MPNIRWBCDMUDJQZN6DDV2HCBRANCNFSM5ZZMU4PA. You are receiving this because you were mentioned.Message ID: @.***>
Just tested it, behaviour looks good! I'm ready for this to be passed by Mike. Assigning to Cyan to pass to him.
@jaydeepsingh25 as discussed in tech-arch today, please wait to merge this until we get initial comments back. @Cybernide please let us know how this is looking, since I'd like to merge earlier rather than later if it is looking positive, to prevent more custom builds.
Contacted our three panelists to ask whether or not they could try the new extension out and then talk to me together, Will update as they get back to me.
Thank you!
From: Cyan @.> Date: Wednesday, August 31, 2022 at 3:40 PM To: Shared-Reality-Lab/IMAGE-browser @.> Cc: Sabrina Knappe @.>, Mention @.> Subject: Re: [Shared-Reality-Lab/IMAGE-browser] remove dropdown (PR #233)
Contacted our three panelists to ask whether or not they could try the new extension out and then talk to me together, Will update as they get back to me.
— Reply to this email directly, view it on GitHubhttps://github.com/Shared-Reality-Lab/IMAGE-browser/pull/233#issuecomment-1233346515, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ANL6MPMQS6NKJPYBRKPSGG3V36YI7ANCNFSM5ZZMU4PA. You are receiving this because you were mentioned.Message ID: @.***>
I've sent instructions to two of the panelists. Will see if they manage.
@Sabrina-Knappe @Cybernide after discussion at tech-arch, is this ready to go? If so, it should be assigned to Jaydeep for merge and deploy.
Assigning to @jaydeepsingh25 to merge!
The PR provides fix for #168.