Feat: add app blockly version #573
Conversation
| STATISTICS, | ||
| PLATFORM, | ||
| COMPILER, | ||
| EMBEDDED_MODE, |
There was a problem hiding this comment.
suggestion: I don't want to create a separate copy of existing code for embedded use, rather make (minimal) changes to the existing code, if needed. This variable is to track, if user is on /embedded path.
|
|
||
| // Only apply mobile layout options when in embedded mode | ||
| if (isEmbedded) { | ||
| blocklyOptions.horizontalLayout = true; |
There was a problem hiding this comment.
suggestion: This are settings to move toolbox to the bottom.
|
|
||
| // Set default locale before any components load | ||
| // This prevents undefined Blockly.Msg properties when blocks are registered | ||
| Blockly.setLocale(De); |
There was a problem hiding this comment.
suggestion: This change is to fix the error I was getting, when I navigated to /embedded directly.
…dd corresponding test
| @@ -0,0 +1,85 @@ | |||
| import React, { useEffect, useRef } from "react"; | |||
There was a problem hiding this comment.
suggestion: I ended up creating a separate toolbox for embedded route to avoid styling mess.
There was a problem hiding this comment.
makes sense to keep it seperated 👍
| <ErrorBoundary> | ||
| <Content /> | ||
| <Switch> | ||
| <EmbeddedRoute path="/embedded" exact> |
There was a problem hiding this comment.
suggestion: This is a route name I thought reflects the best it's purpose. But I am open for suggestions.
…edded route was created
Thiemann96
left a comment
Thiemann96
left a comment
There was a problem hiding this comment.
I tested it with the preview link on my smartphone. If there is an app version let me know.
I am not sure how I like the toolbox on the bottom or top to be honest.
When scrolling through the selected category sometimes you "click" the block. I think this is because the blocks fill out almost the entire flyout and the user has no space to scroll. Moreover I (personally) dislike sideway scrolling. It feels weird to me and leads to errors like the one mentioned above.
I think for example OzoBlockly has some interesting idead about mobile navigation of blockly , i.e. forcing portrait mode and having the flyout be almost as big as the screen so scrolling is not always neccessary. Additional examples which use Blockly and have a mobile section : Microsoft MakeCode, or OpenRobertalLab.
We can also decrease the size of blocks to further prevent scrolling if possible. And I think you mentioned it before but the sub categories are kinda buggy. I think finding a solution for this on mobile will be hard without cluttering the screen and I think we can firstly "ignore" sub categories and just put them in one.
Overall though this is a very good proposal and definetly a step in the right direction.
Let me know what you think, I am definetly open for discussion and interested to hear what you think / thought during testing.
| // Inject Blockly once on mount | ||
| useEffect(() => { | ||
| const ws = Blockly.inject(blocklyDivRef.current, { | ||
| const blocklyOptions = { |
There was a problem hiding this comment.
i could not find exactly where but I think the renderer for blockly got changed. Im guessing that this was not intentional. thats why the blocks look different. i think putting the renderer "Thrasos" in the blockly options makes the blocks look "normal" again.
| @@ -0,0 +1,85 @@ | |||
| import React, { useEffect, useRef } from "react"; | |||
There was a problem hiding this comment.
makes sense to keep it seperated 👍
Here it is https://github.com/sensebox/sensebox-connect/pull/17/. PR turned to draft i guess because it couldn't be merged in main after you merged previous PR. |
Tests
Github Test Reporter by CTRF 💚 |
Changes
Open questions