Music Play View - part 2 #58293

fisher-alice · 2024-04-26T22:31:11Z

This PR updates the Music Lab play view and follows up #58068.

Note that the adhoc based on this branch is available at https://adhoc-alice-play-view-updates-studio.cdn-code.org/home.
Example project in play view.

Updates in this PR include:

Adds MusicPlayView to MusicLabView so that when isShareView is set to true, the play view is rendered.
In MusicView, when isPlayView is set to true, a headless musicBlocklyWorkspace is initialized, i.e., the UI of the blockly workspace is not rendered. Thanks to @sanchitmalhotra126 for pairing with me on this! I based this part of the PR on Sanchit's WIP branch.
Adds functionality to the play/stop slider and shows progress as song plays.
Adds functionality to the 'View code' button (view/edit mode) and Make my Own' button (remix).
Adds redirectToView to projectsApi.
Adds the music play image from product.
Updated styling of play view based on product/design preferences.

The 'Share' button in play view is displayed on most mobile devices. But you can view this button if you add the query param '?canShare=true' to the end of the project play view url, e.g., https://adhoc-alice-play-view-updates-studio.cdn-code.org/projects/music/lpef7b3VcqvntTmBLG2c1w?canShare=true

After Update

Play view card on desk top:

Play view full screen on desk top:

Play view on mobile:

Screencast video of play view on desktop:

demo-design.mp4

Links

Testing story

Deployment strategy

Follow-up work

Follow-up will be to refactor the slider and include adding color to progress.

Privacy

Security

Caching

PR Checklist:

Tests provide adequate coverage
Privacy and Security impacts have been assessed
Code is well-commented
New features are translatable or updates will not break translations
Relevant documentation has been added or updated
User impact is well-understood and desirable
Pull Request is labeled appropriately
Follow-up work items (including potential tech debt) are tracked and linked

levadadenys

LGTM overall! 😍
Couple non blocking comments.
Not approving only because PR is in Draft state

levadadenys · 2024-04-29T17:19:17Z

apps/src/music/views/music-play-view.module.scss


      input[type='range'] {
        overflow: hidden;
        width: 100%;
-        height: 4px;
-        accent-color: $light_gray_200;
+        accent-color: yellow;


please note: worth checking how it looks like in different browsers, since this won't affect all of the browsers. (when using $light_gray_200 it's just not that visible so to say, but still there's a difference between different browsers)

https://css-tricks.com/styling-cross-browser-compatible-range-inputs-css/

https://www.cssportal.com/style-input-range/

Thanks so much! I've been noticing the differences so I appreciate the resources.

sorry, I've duplicated the links in the initial comment. Updated it. added https://www.cssportal.com/style-input-range/ which allows to generate cross-browser styles for input type range more easily

apps/src/music/views/MusicPlayView.tsx

fisher-alice · 2024-04-29T19:58:37Z

dashboard/app/controllers/projects_controller.rb

    return nil unless @level.uses_lab2?
-
+    # If the user is on the play view '/projects/channel_id', set `response_content`.`
+    if params[:share] == true


Not sure if this is okay to do this for lab2 labs or should I only do for music lab projects for now? @sanchitmalhotra126

fisher-alice · 2024-04-30T01:13:42Z

apps/src/music/views/music-play-view.module.scss

-      gap: 0.5rem;
+      gap: 1rem;
+
+      input[type=range] {


Reference: https://css-tricks.com/styling-cross-browser-compatible-range-inputs-css/
Style range inputs so that styling is consistent across Firefox, Edge, Safari, and Chrome.

Cool stuff!

Slick, need this for AI Chat too!

fisher-alice · 2024-04-30T23:06:11Z

dashboard/app/controllers/projects_controller.rb

+    return nil unless @level.uses_lab2?
+
+    # If the user is on the play view '/projects/channel_id', set `response_content`.`
+    if params[:share] == true


This is checked in application.html.haml here.

Wondering if we should technically use the sharing value computed on line 374 so that we're consistent with what we pass into level_view_options. It shouldn't matter for now since we don't do anything special for embedded projects in lab2 currently, but just to be safe? Curious if @breville has thoughts

Yeah - I was wondering about this earlier. Thanks! Will wait on Brendan to weigh in.

Discussed offline - I'll go ahead and update to use sharing.

sanchitmalhotra126

Looks pretty good overall! A few comments/questions

apps/src/lab2/views/Lab2Wrapper.module.scss

apps/src/mixins.scss

apps/src/music/views/MusicPlayView.tsx

sanchitmalhotra126 · 2024-04-30T23:26:06Z

apps/src/music/views/music-play-view.module.scss

-      gap: 0.5rem;
+      gap: 1rem;
+
+      input[type=range] {


Cool stuff!

sanchitmalhotra126 · 2024-04-30T23:29:32Z

dashboard/app/controllers/projects_controller.rb

+    return nil unless @level.uses_lab2?
+
+    # If the user is on the play view '/projects/channel_id', set `response_content`.`
+    if params[:share] == true


Wondering if we should technically use the sharing value computed on line 374 so that we're consistent with what we pass into level_view_options. It shouldn't matter for now since we don't do anything special for embedded projects in lab2 currently, but just to be safe? Curious if @breville has thoughts

sanchitmalhotra126

A few minor things but LGTM! Nice work!

apps/src/lab2/views/Lab2Wrapper.tsx

apps/src/mixins.scss

apps/src/music/views/MusicPlayView.tsx

apps/src/music/views/music-play-view.module.scss

fisher-alice added 13 commits April 26, 2024 21:27

add redirectToView

d79bf01

add overlay to lab2wrapper if shareview

28cfa4d

add image, add playview to musiclabview

8d360e1

add default value for progrgressSliderValue for nan

a7623db

add readonly to input

5a8400f

use state for slider value

3c46039

add disabled isLoading

f4d1b5e

add className and tweak styling

e2c4db1

remove unnec styling

cb9aa2d

update view_options (response_content) when on play view

588e3f8

update comment

0556772

play section has darker background

b1c6762

revert darker background for now

6df403f

levadadenys reviewed Apr 29, 2024

View reviewed changes

update share button style

01060c8

fisher-alice commented Apr 29, 2024

View reviewed changes

fisher-alice added 2 commits April 30, 2024 00:30

update slider style so works across multiple browsers

de2cd14

change to buttons to tertiary

0d9cf05

fisher-alice commented Apr 30, 2024

View reviewed changes

fisher-alice added 4 commits April 30, 2024 16:04

small style updates

5b6af45

change play background, button, re-align button section

d2184c8

more design updates

ff7f4c2

create separate function to set view_options

4923cc3

fisher-alice commented Apr 30, 2024

View reviewed changes

fisher-alice marked this pull request as ready for review April 30, 2024 23:06

fisher-alice requested review from a team and sanchitmalhotra126 April 30, 2024 23:07

sanchitmalhotra126 reviewed Apr 30, 2024

View reviewed changes

fisher-alice added 2 commits May 1, 2024 18:01

clean up styles, review feedback

1d3ded1

add labContainerShareView style

e8e4834

remove unused style rule

efecfdf

fisher-alice requested a review from sanchitmalhotra126 May 1, 2024 18:42

sanchitmalhotra126 approved these changes May 1, 2024

View reviewed changes

review feedback

9f5280c

fisher-alice merged commit a50c045 into staging May 1, 2024
2 checks passed

fisher-alice deleted the alice/play-view-updates branch May 1, 2024 23:26

fisher-alice mentioned this pull request May 2, 2024

Add Music lab progressSlider component #58373

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Music Play View - part 2 #58293

Music Play View - part 2 #58293

fisher-alice commented Apr 26, 2024 •

edited

levadadenys left a comment

levadadenys Apr 29, 2024 •

edited

fisher-alice Apr 29, 2024

levadadenys Apr 30, 2024

fisher-alice Apr 29, 2024

fisher-alice Apr 30, 2024

sanchitmalhotra126 Apr 30, 2024

bencodeorg May 1, 2024

fisher-alice Apr 30, 2024

sanchitmalhotra126 Apr 30, 2024

fisher-alice May 1, 2024

fisher-alice May 1, 2024

sanchitmalhotra126 left a comment

sanchitmalhotra126 Apr 30, 2024

sanchitmalhotra126 Apr 30, 2024

sanchitmalhotra126 left a comment

Music Play View - part 2 #58293

Music Play View - part 2 #58293

Conversation

fisher-alice commented Apr 26, 2024 • edited

After Update

Links

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

levadadenys left a comment

Choose a reason for hiding this comment

levadadenys Apr 29, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanchitmalhotra126 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanchitmalhotra126 left a comment

Choose a reason for hiding this comment

fisher-alice commented Apr 26, 2024 •

edited

levadadenys Apr 29, 2024 •

edited