Music: standalone video levels by breville · Pull Request #52454 · code-dot-org/code-dot-org

breville · 2023-06-26T19:37:33Z

This change began because our curriculum team now prefers videos to be on separate levels to labs, and so the Music lab progression needed to support interspersed video levels.

We decided to leverage the existing StandaloneVideo level type, which has useful backend and levelbuilder support. But we built a new client to use when these levels are interspersed with Music Lab levels; it supports switching levels without a page reload, and is React-based.

At the same time, we settled on Lab2 as the name for the collection of client-side improvements we've been making. This change introduces a bunch of new functionality for Lab2 levels:

Music levels have the uses_lab2 property, and StandaloneVideo levels can use it as well.
When the user visits a ScriptLevel that has the uses_lab2 property, we load the client-side code for a new lab2 app, which creates a Lab2 React component at the root of the lab area.
The Lab2 component now examines all levels in the current lesson and manages the lifecycle of each relevant app. A centralized list allows each app to have some custom behaviour.
- For example, a StandaloneVideo app is added and then removed from the DOM as a level is visited or departed, while a Music app is shown or hidden as a level is visited or departed.
LabContainer has been renamed to Lab2Wrapper and continues to provide several functions, including an error boundary, inter-level fades, and a sad bee in case of a serious error.
The system now permits level switches without a page reload so long as the current and the destination level both have the property uses_lab2.

standalone-video-no-reload-3.mov

Many thanks to @sanchitmalhotra126 and @molly-moen for engaging with this PR as it evolved. It felt like a form of asynchronous pair programming via pull request, and had a huge impact on how this work progressed.

molly-moen · 2023-06-26T19:55:48Z

    @@game_standalone_video ||= find_by_name("StandaloneVideo")
  end

+  def self.standalone_video2


can we think of a better name than standalone_video2?

I tried for a while, and am open to ideas. I thought about video (but then the model collides with an existing Video class in the dashboard), VideoLab (since it has characteristics of a lab, but it isn't really a lab), and VideoLevel (but then naming of related files gets weird) for starters. This really will look a lot like a standalone_video level, but it will be React-based with a proper JS app backing it, and with support for level changes without page reloads.

After seeing how much server-side support we have built for StandaloneVideo, including levelbuilder support, and our discussion about the disadvantages of adding another model and game type, I'm trying a different approach: we'll use StandaloneVideo on the server, but when we see it in use alongside Music Lab levels, we'll use a newer client-side implementation (name to be determined) which is React-based and supports changing levels without reloads.

molly-moen · 2023-06-26T20:00:59Z

    <ErrorBoundary fallback={<ErrorFallbackPage />} onError={onError}>
      <div id="lab-container" className={moduleStyles.labContainer}>
-        {children}
+        <ProjectContainer channelId={getStandaloneProjectId()}>


I think this file will get way too big and confusing if we have to put every lab's configuration in here. At least for now it would be good to get each lab down to 1 line (so we would extract out MusicView wrapped by ProjectContainer into some other component), and have each one look like the line for StandaloneVideo2

+1. It might be nice to store some map of appName -> React class separately, so we can just look it up there and render the desired component here

Agreed on both points. For starters, it's lab-agnostic again.

molly-moen · 2023-06-26T20:01:11Z

+            style={{
+              width: '100%',
+              height: '100%',
+              visibility: currentApp !== 'music' && 'hidden',


why are we using visibility here rather than conditionally rendering?

I tried using conditional rendering but then ran into a problem I also encountered in earlier explorations: we don't seem to have a clean way to tear down Blockly and then recreate it. If we stop rendering the lab, and then start again, we'll attempt to recreate Blockly, but hit a bunch of errors because it has left things behind on the window.

molly-moen · 2023-06-26T20:02:05Z

+// without reloading the whole page.
 export function canChangeLevelInPage(currentLevelApp, newLevelApp) {
-  return currentLevelApp === 'music' && newLevelApp === 'music';
+  const compatibleApps = ['music', 'standalone_video2'];


this logic feels like it is opening us up to allowing switches between any app type. Do we want that? It might be fine, it just feels like a departure from our previous thoughts.

I expect a more likely situation is that we "family up" the sets that we can switch between, and had always kept that in the back of my mind. We still restrict these switches to those within the same lesson, so I think it makes sense that we can switch between more than one app type, but only a known subset within that given lesson.

molly-moen · 2023-06-26T20:02:56Z

-    if (prevProps.currentLevelIndex !== this.props.currentLevelIndex) {
+    if (
+      prevProps.currentLevelIndex !== this.props.currentLevelIndex &&
+      this.props.isActive


if we switch to conditional rendering do we need to worry about isActive?

We didn't need it in the end.

molly-moen · 2023-06-26T20:04:50Z

+
+# StandaloneVideo2 uses level_data, which is actual JSON in each .level file.
+
+class StandaloneVideo2 < Level


should StandaloneVideo2 extend from StandaloneVideo?

I think the server-side model should be different between them, since StandaloneVideo2 would start to rely on a level_data JSON blob rather than the separate serialized attributes used in StandaloneVideo, and probably doesn't need to inherit all of the older model's methods.

As mentioned elsewhere, this change no longer adds a new model on the server, and instead just adds a new client for standalone video levels that are shown in a LabContainer context.

sanchitmalhotra126 · 2023-06-26T23:53:15Z


 const MusicView = connect(
  state => ({
+    isActive:


If we do want to go this route, I think this prop should be passed in at the top level by LabContainer directly, or this calculation should get encapsulated in a selector somewhere

Agreed. At least for now, it doesn't seem to be necessary.

sanchitmalhotra126 · 2023-06-26T23:55:30Z

+import {logError} from '@cdo/apps/music/utils/MusicMetrics';
+import {getStandaloneProjectId} from '@cdo/apps/labs/projects/utils';
+
+$(document).ready(function () {


Interesting, so seems like we will still need to create one of these entry points for any new lab, but can then switch to any other lab from there?

Yes, that is the principle. It's still early on this, but I think we could use a new, common piece of code when either of these level types is encountered in a script level.

That common piece of code is now in place.

molly-moen · 2023-06-30T19:34:31Z

              </p>
            </HelpTip>
          </label>
+          <label>


We probably don't want levelbuilders to have to know this, right? Maybe this should just be a level property?

+1, discussed a bit offline but seems setting this on the level (or the game?) would be better. I'm wondering if we can even go so far as storing the list of lab-lab compatible app types on the backend (like some list of game&.lab_lab_enabled?), and then send that list down to the front end, so that level-switching code doesn't need to store its own list of compatible apps.

This is now set on the level. For music levels, uses_lab_container? will always be true. For standalone_video levels, we can mark it true using a serialized attribute for the specific levels that appear alongside music levels. A bonus is that the level-switching code no longer stores its own list of compatible apps, and instead just checks whether the user is moving between two levels that use LabContainer.

molly-moen · 2023-06-30T19:34:54Z

@@ -0,0 +1,60 @@
+import $ from 'jquery';


I like having a lablab entrypoint 🎉

+1! one thing that comes to mind is handling locale files - maybe the entrypoint just loads all the locale files for lab-lab compatible apps? don't know how large that would be though.

For now that's exactly what is happening, in _lab2.html.haml.

molly-moen · 2023-06-30T19:35:56Z

+  );
+});
+
+const LabLab = () => {


can we move this to another file since it would probably be nice to have it in a tsx file? It also would ensure we keep the entrypoint small

Good call; done.

molly-moen · 2023-06-30T19:43:55Z

    @app_options =
-      if @level.is_a? Blockly
+      if @script_level&.lesson&.use_lab_container
+        {app: 'lablab'}


are we removing some app options by doing this? I believe the only one we are relying on is channel, but we want to make sure we are still sending it.

Oh, do we still need channel ID rendered into the page?

Yeah, we will for standalone projects

Got it. I think I've now got that working.

sanchitmalhotra126 · 2023-06-30T19:57:20Z

  startSources: Source;
 }

+export interface LevelVideo {


Can this be part of levelData so we don't need to store it separately?

I guess ideally this would be a variant type of level data. To that end, I've prepped a standalone PR (which builds on this one) which shows one idea for that. Would love thoughts: #52570

Merged into this PR.

sanchitmalhotra126 · 2023-06-30T20:00:10Z

              </p>
            </HelpTip>
          </label>
+          <label>


+1, discussed a bit offline but seems setting this on the level (or the game?) would be better. I'm wondering if we can even go so far as storing the list of lab-lab compatible app types on the backend (like some list of game&.lab_lab_enabled?), and then send that list down to the front end, so that level-switching code doesn't need to store its own list of compatible apps.

sanchitmalhotra126 · 2023-06-30T20:00:57Z

@@ -0,0 +1,60 @@
+import $ from 'jquery';


+1! one thing that comes to mind is handling locale files - maybe the entrypoint just loads all the locale files for lab-lab compatible apps? don't know how large that would be though.

sanchitmalhotra126 · 2023-06-30T20:03:56Z

+});
+
+const LabLab = () => {
+  const currentApp = useSelector(


Can we get this from the backend directly? i feel a little wary of relying on progress to determine the app type - i'm not sure in what situation we'd be switching levels without progress, but feels like it would be better to decouple ourselves given that we're already retrieving other level data from the backend?

It's a good point. The challenge is that we would like to have this information for all levels available up front so that we can start each switch instantly, and then the per-level level data is retrieved. So far, we've been using progressRedux as our source of truth for the set of levels, so it feels like a natural fit to also store the app type there. The bigger change would be to factor information for the available set of levels into a more general redux store.

breville · 2023-07-01T21:32:45Z

+    currentApp === 'music' ? getStandaloneProjectId() : null;
+
+  return (
+    <ProjectContainer channelId={channelId || undefined}>


getStandaloneProjectId returns string | null but ProjectContainer wants string | undefined. Should we make these consistent?

Yeah, string | undefined should be fine, especially after Molly's change here since we're no longer explicitly returning null.

Great; I'll update the return value type of getStandaloneProjectId and tidy this up.

molly-moen

This is looking good! Couple questions/potential follow-ups

molly-moen · 2023-07-05T17:47:04Z

+        logError({error: error.toString(), componentStack})
+      }
+    >
+      <ProjectContainer channelId={channelId}>


should we rename ProjectContainer to be a little more generic? Can be done separately. Now it loads project and or level data, which is sort of a confusing thing to name.

Agreed. I think best if that's done in a follow-up by someone who's more familiar with it!

molly-moen · 2023-07-05T17:47:48Z

+                style={{
+                  width: '100%',
+                  height: '100%',
+                  visibility: currentAppName === 'music' ? 'visible' : 'hidden',


will we no longer need to do the visibility hiding after Sanchit's PR? #52588

Quite possibly, yes. I'm still curious whether we might decide it's better just to adjust visibility in some cases to cut down on reloads of sounds in Music Lab.

molly-moen · 2023-07-05T17:58:22Z

+  # the level, which usually include levelData.  If this level is a
+  # StandaloneVideo then we put its properties into levelData.
+  def summarize_for_lab2_properties
+    video = specified_autoplay_video&.summarize(false)&.camelize_keys


would it be possible here to add a type property (ex. for Music is would be 'music', for Video it would be 'video'? This may make our lives easier on the frontend given we are using a union type, and will be adding to it going forward.

I can add it here, though I'm not sure what to do with it on the front-end apart from double-checking that it matches expectations and maybe logging some kind of error if it isn't?

It will help with type checking--if we know something has a music type we then will know which other fields it has. But it's not strictly necessary, it just could give us cleaner code down the line.

Sure thing. As a note, I used the type property from Level, which comes through as "Music" or "StandaloneVideo".

sanchitmalhotra126

A few more smaller things but looking good!

sanchitmalhotra126 · 2023-07-05T18:41:45Z

+  const currentAppName: string = currentLessonLevels.find(
+    (level: LevelWithProgress) => level.isCurrentLevel
+  ).app;
+  const currentLessonAppNames: string[] = Array.from(


Worth moving currentLessonAppNames into a selector in progressReduxSelectors, if it would be useful to reuse?

I might leave this for a potential follow-up, since it feels a bit specific to this logic.

sanchitmalhotra126 · 2023-07-05T18:42:31Z

+                style={{
+                  width: '100%',
+                  height: '100%',
+                  visibility: currentAppName === 'music' ? 'visible' : 'hidden',


This should be currentAppName === appProperty.name right?

Oops, thanks.

sanchitmalhotra126 · 2023-07-05T18:43:33Z

+                  visibility: currentAppName === 'music' ? 'visible' : 'hidden',
+                }}
+              >
+                <ProgressContainer appType={appProperty.name}>


To help simplify this, I think we can move ProgressContainer up into set of common wrapper components. If the app doesn't use progress, it just won't call any functions on the progress manager and won't respond to progress manager state updates.

Oh that's great, and helps simplify things.

sanchitmalhotra126 · 2023-07-05T18:44:43Z

+            );
+          } else if (appProperty.name === currentAppName) {
+            return (
+              <div


Do we need to wrap this in a separate div? Can we just render the app view directly?

I wanted a way to get a consistently-named div for any type of app, which has proven pretty useful for seeing things working in the DOM like this:

Screen.Recording.2023-07-05.at.1.25.39.PM.mov

sanchitmalhotra126 · 2023-07-05T18:47:27Z

+  return (
+    <Lab2Wrapper
+      onError={(error, componentStack) =>
+        logError({error: error.toString(), componentStack})


Just noting this for a later follow-up: logError is a music-specific logging function, so we should instead have more generic logging here (can be done as part of https://codedotorg.atlassian.net/browse/SL-918)

sanchitmalhotra126 · 2023-07-05T18:49:20Z


 $(document).ready(function () {
  ReactDOM.render(
    <Provider store={getStore()}>


Can we also replace this with <Lab2>? Or are we not even hitting this entrypoint anymore?

We still hit this entrypoint when viewing an individual level using a /levels/[level_id] URL.

sanchitmalhotra126 · 2023-07-05T18:51:23Z

@@ -0,0 +1,49 @@
+// StandaloneVideo


minor nit: left a similar comment on Molly's PR, but should we standardize where we put top-level docs for react functional components? i have a slight preference for putting them after imports, on top of the component declaration, but could go either way.

I also don't have strong feelings, but I think there is one advantage to having them at the top of the file.

sanchitmalhotra126 · 2023-07-05T18:52:20Z

+  targetWidth -= 80;
+  targetHeight -= 100 + childrenAreaHeight;


nit: constants and/or docs for some of these adjustment values?

Good call; updated.

sanchitmalhotra126 · 2023-07-05T18:53:38Z

+      if @script_level && @level.uses_lab2?
+        {app: 'lab2', channel: view_options[:channel]}


Will we not use lab2 for non-script single levels (levels/xyz urls)?

It's a good question. The Lab2 component right now has some assumptions that it's part of a ScriptLevel, since it examines the current lesson and has logic for loading multiple apps at once. Maybe it should be renamed to Lab2ScriptLevel or similar?

I'm definitely open to more comprehensive refactoring, but this change is also getting big enough that I wouldn't mind getting it merged, and then we can do more refactoring as follow-ups.

What do you think?

Gotcha - yeah I think it would be nice to reduce/remove the dependency on progress redux so that Lab2 can be used for any lab on any page, but agreed that it can happen as a follow up. From what i'm seeing, I think if we were to pass the app type in as part of level properties, we shouldn't need to look up app type on the lesson, right? As far as loading multiple app views - it would be interesting to see how many apps need to stay rendered vs how many can be unmounted, but if it there aren't many that need to stay rendered, I think we can get away with handling multiple renders for all app types, rather than ones just in the lesson.

From what i'm seeing, I think if we were to pass the app type in as part of level properties, we shouldn't need to look up app type on the lesson, right?

While we'd know the app type of the current level, we'd still want to know the app type of the destination level ahead of time, so that we can switch immediately to it and then initiate the retrieval of its own level properties.

Right yeah, but that's only for level switching right? Within the Lab2 container, would it be feasible to only worry about rendering the current app? In the case of unmountable apps like music, we can have some logic that only renders them the first time we encounter them, and then keeps them hidden on the page.

Again, totally fine with this approach, just want to see how flexible we can make this

Oh, that's an interesting idea. I guess the current technique effectively "preloads" Music Lab so that it's already in the DOM, just not visible, but with this idea we are more opportunistic about what we load. Makes sense, and worth exploring!

sanchitmalhotra126 · 2023-07-05T18:55:38Z

+  %script{src: webpack_asset_path("js/#{js_locale}/music_locale.js")}
+  %script{src: webpack_asset_path("js/#{js_locale}/standaloneVideo_locale.js")}
+  %script{src: webpack_asset_path('js/googleblockly.js')}
+  = render_app_dependencies


Do we still need render_app_dependencies here since we're already loading blockly and locale?

Good question; it was possible to remove this dependency.

sanchitmalhotra126

Looks great! Thanks for all the work on this! Lots of exciting changes here, great to see these come in.

Just wanted to round up some of the follow-ups that were noted across various comments (I'll create JIRAs for these, please add if I missed any):

Rename ProjectContainer to something more generic
Evaluate ummounting Music Lab vs keeping it mounted and hidden
Replace logError with generic logging function in Lab2Wrapper (can be part of https://codedotorg.atlassian.net/browse/SL-918)
Investigate using Lab2 on non script-level pages
Move currentLessonAppNames to progressReduxSelectors
Loop through all lab2 apps to include locale files in lab2.html.haml

sanchitmalhotra126 · 2023-07-05T22:58:20Z

            <div className={moduleStyles.slowLoadContainer}>
              <div className={moduleStyles.spinnerContainer}>
                <FontAwesome
+                  title={undefined}


What's this for?

It comes from here. It complained that we weren't setting it, but I'm wasn't sure that baking in a string (which should be localized) would actually be useful for a screenreader, so this is a half-measure.

Can you add a comment explaining that we should add a localized title?

This was a deliberate choice, but I think there might be more we can do to make spinners accessible. I'll open a follow-up item.

sanchitmalhotra126 · 2023-07-05T22:59:14Z

+interface AppProperties {
+  name: string;
+  usesChannel: boolean;
+  usesProgressManager: boolean;


Can we remove this property?

Looks like it. Removed.

sanchitmalhotra126 · 2023-07-05T23:19:45Z

+import Lab2 from '@cdo/apps/labs/views/lab2';
+
+$(document).ready(function () {
+  ReactDOM.render(<Lab2 />, document.getElementById('lab2-container'));


sanchitmalhotra126 · 2023-07-05T23:29:02Z

  #  3. The disable_google_blockly DCDO flag, which contains an array of strings corresponding to model class names.
  #     This option will override #2 as an "emergency switch" to go back to CDO Blockly.
  def use_google_blockly
+    return true if @level.uses_lab2?


Should we default to the level setting here? This won't always be true when we have non-Blockly labs right? Though FWIW since we're not depending on apps_dependencies do we even check this?

It seems we're no longer using this, so I've just removed the line.

molly-moen

🎉 this looks great!

molly-moen · 2023-07-06T15:36:40Z

            <div className={moduleStyles.slowLoadContainer}>
              <div className={moduleStyles.spinnerContainer}>
                <FontAwesome
+                  title={undefined}


Can you add a comment explaining that we should add a localized title?

Music: standalone video levels (WIP)

9828d68

breville requested a review from a team as a code owner June 26, 2023 19:37

breville marked this pull request as draft June 26, 2023 19:37

molly-moen reviewed Jun 26, 2023

View reviewed changes

sanchitmalhotra126 reviewed Jun 26, 2023

View reviewed changes

breville added 14 commits June 27, 2023 04:16

Merge branch 'staging' into music-standalone-video-levels

7a447e0

Merge branch 'staging' into music-standalone-video-levels

574ecf8

Move away from a new video_standalone2 model

08d2fa6

Merge branch 'staging' into music-standalone-video-levels

02f327b

Small changes

50112e6

Merge branch 'staging' into music-standalone-video-levels

3b0955d

A standalone_video now shows without a reload

788549c

Can load video level page and jump around

2a4870d

Add use_lab_container to lesson properties

a121fa5

Add background color to music video

6bf6474

Merge branch 'staging' into music-standalone-video-levels

e8ca99c

Some tidying

e4fe67a

Levels & script levels now work

8e5966d

Updates

159a542

molly-moen reviewed Jun 30, 2023

View reviewed changes

sanchitmalhotra126 reviewed Jun 30, 2023

View reviewed changes

breville added 3 commits July 1, 2023 20:07

lablab -> lab2 & some refactoring

99f4b28

Game or level can declare it uses lab container for initial load

d39796a

Load properly for levels & script levels

39058fa

breville commented Jul 1, 2023

View reviewed changes

breville added 4 commits July 1, 2023 21:42

Generalize check for navigation without reload

ae02af0

StandaloneVideo2 -> StandaloneVideo

cd05615

Video player gets localized next button

ff3bfb4

Move video to first level

9072767

breville added 5 commits July 4, 2023 01:10

Remove standaloneVideo from app build

97593b5

Remove unused file

acb5ee0

LabContainer -> Lab2Wrapper

0d046bf

Update comment

a9a141d

Merge branch 'staging' into music-standalone-video-levels

5c60729

molly-moen reviewed Jul 5, 2023

View reviewed changes

sanchitmalhotra126 reviewed Jul 5, 2023

View reviewed changes

breville added 9 commits July 5, 2023 19:22

Moce some files

655b9b0

Lab2Wrapper.jsx -> Lab2Wrapper.tsx

17bf551

Return type in level_properties

70f4d6e

Fix check

efc4cbb

Move ProgressContainer in DOM

dc1c6cb

Use constants for video sizing

96f57c8

Remove dependency on app_dependencies

fbb7e52

Lab2 provides its own Provider

5cf06b6

Merge branch 'staging' into music-standalone-video-levels

555fa0a

breville marked this pull request as ready for review July 5, 2023 22:50

breville requested review from a team and moneppo July 5, 2023 22:50

breville changed the title ~~Music: standalone video levels (WIP)~~ Music: standalone video levels Jul 5, 2023

sanchitmalhotra126 approved these changes Jul 5, 2023

View reviewed changes

breville added 3 commits July 6, 2023 00:13

Remove unused field

a1f4858

Remove unused code

a988886

Fix unit tests

a081475

molly-moen approved these changes Jul 6, 2023

View reviewed changes

breville merged commit c32bf70 into staging Jul 6, 2023

breville deleted the music-standalone-video-levels branch July 6, 2023 15:51

breville mentioned this pull request Jul 7, 2023

Music: restore video for incubator version #52693

Merged

ebeastlake mentioned this pull request Jul 11, 2023

render using lab2 framework #52730

Merged

breville mentioned this pull request Sep 7, 2024

Lab2: standalone video fallback player #60903

Merged


		# StandaloneVideo2 uses level_data, which is actual JSON in each .level file.

		class StandaloneVideo2 < Level

		if @script_level && @level.uses_lab2?
		{app: 'lab2', channel: view_options[:channel]}

Conversation

breville commented Jun 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

breville Jun 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

breville Jul 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

breville commented Jun 26, 2023 •

edited

Loading

breville Jun 27, 2023 •

edited

Loading

breville Jul 1, 2023 •

edited

Loading