search

source-academy / frontend

Frontend of Source Academy, an online experiential environment for computational thinking (React, Redux, Saga, Blueprint)
https://sourceacademy.org
Apache License 2.0
103 stars 168 forks source link

Language selector bypasses restrictions set on language chapters #2491

Open ianyong opened 1 year ago

ianyong commented 1 year ago

This allows Folder mode to be accessed in Source 1. firefox_A52lqKGyth

Steps to replicate:

  1. Switch to Source 2+ in the chapter selector.
  2. Switch to Python/Scheme in the language selector.
  3. Switch back to JavaScript in the language selector.
ianyong commented 1 year ago

Similar to #2465 except no page reloads are necessary to break the invariant that Source 1 cannot be used with Folder mode.

RichDom2185 commented 1 year ago

Similar to #2465 except no page reloads are necessary to break the invariant that Source 1 cannot be used with Folder mode.

Hi, I don't think the two issues are similar. The root cause of that issue is more of it takes a while to persist the updated store after a state update. The root cause of this one is flawed business logic:

https://github.com/source-academy/frontend/blob/a231c3b883e85827bc09a3cc27d41f1b6a128786/src/commons/navigationBar/subcomponents/NavigationBarLangSelectButton.tsx#L17-L24

This assumes that components that depend on the current language (e.g. Playground) would respect this and always only provide the appropriate features (e.g. Folder Mode) accordingly.

But in this case, there is no additional check in Playground to dispatch toggleFolderMode should isFolderModeEnabled === true and the language changes to something that does not support folder mode. This is because Playground is operating under the false assumption that isFolderModeEnabled will always be initially consistent with whether the current language supports it or not.

In fact, the issue has nothing to do with Source 1 specifically. Notice how in your screenshot, when you switched to Scheme or Python (which is also defined to not support multi-file), it exhibits the same behaviour as changing to Source 1.

I think the current solution for this is to do the same thing you did: disable the language dropdown when using folder mode (but that not might be the proper thing to do...)

ianyong commented 1 year ago

This is because Playground is operating under the false assumption that isFolderModeEnabled will always be initially consistent with whether the current language supports it or not.

This is a consequence of the design choice that Folder mode must always be toggled explicitly by the user (in the sense that we cannot not have this assumption if we want to maintain this design choice). This is because Prof @martin-henz and I felt that Folder mode being automatically toggled off when the user switches to Source 1 for example is unintuitive and confusing.

I think the current solution for this is to do the same thing you did: disable the language dropdown when using folder mode (but that not might be the proper thing to do...)

It might make more sense for each language in the language selector to be treated separately? I say this because Folder mode is not well defined for Python and Scheme. In other words, the state of the workspace per language should be saved somewhere instead of defaulting to a sublanguage. In addition, features such as Folder mode as well as remote execution (which if I'm not wrong does not work for Python and Scheme) should not appear for these languages.

Honestly, the implementation of the language selector is not well thought out. I think something along the lines of what I suggested above might be necessary to manage the complexity introduced with multiple languages. Otherwise if we visualise the playground's states as the set of all possible configurations (language, Folder mode, etc.), we would end up with unmanageable numbers of possible state transitions (which are in essence what we care about when we disable sublanguages while Folder mode is enabled) as more languages and sublanguages are added to the Source Academy in the future.

RichDom2185 commented 1 year ago

This is a consequence of the design choice that Folder mode must always be toggled explicitly by the user (in the sense that we cannot not have this assumption if we want to maintain this design choice). This is because Prof @martin-henz and I felt that Folder mode being automatically toggled off when the user switches to Source 1 for example is unintuitive and confusing.

I understand and I agree, it should be toggled independently from the language selection, but it does mean that if the frontend can't guarantee the consistency all the time (such as in this case), then things may break unexpectedly.

In addition, features such as Folder mode as well as remote execution (which if I'm not wrong does not work for Python and Scheme) should not appear for these languages.

Folder mode is already hidden for these languages as languageConfig.supports.multiFile is false here. Regarding remote execution, I'm planning to modularise it (#2412), because we are trying to support more types of robots beyond EV3. In the case of SPIKE Prime (another robot), it supports both Python and Source, as demonstrated in https://github.com/RichDom2185/seker. This makes the logic even more complicated as now depending on what the device type is, some languages may or may not be supported. As of now, it is still hardcoded (and locked) to Source 3 whenever a device is connected.