core: pod update #62
core: pod update #62
Conversation
1. move tool bar to right 2. modify output effect 3. add success hint 4. add pod index hint 5. modify pod ui
1. toolbox position 2. layout and delete btn
|
Thanks for your awesome work. Many bugs in my code are fixed in your PR, thanks a lot. I really appreciate your implementation. However, I fetched this branch and tested, some tiny issues show up on my side but I'm not sure what cause them. Can you help me investigate them?
|
|
@li-xin-yi ok, i will check this |
|
@lihebi @li-xin-yi |
@forrestbao Thanks for the advices. I meant to use the box shadow and its color to show which user is dragging or selecting the pod, which are of the same color with the remote cursor. This is also the awareness feature in collaborative editing. I know it may look awkward, but I have no idea how to show the awareness info for dragging/selecting operation upon a pod in a proper form. Can you give me any suggestion? BTW, in my experimental demo (https://github.com/li-xin-yi/sharing-edited-monaco-flow), I convey the awareness and movement information by setting the pod half-transparent and changing its background color. But it seems not so suitable in our project. |
@li-xin-yi Ah, i didn't notice the shadowing is for dragging indication. Maybe we can use border highlighting, and potentially bolding, for both code editing and dragging?
This actually might be a good idea. Maybe we shall have a meeting to brainstorm different ideas to get users attention? |
|
@li-xin-yi the issues you mentioned fixed |
| pod.result.text && ( | ||
| <Box> | ||
| <Box sx={{ display: "flex" }} bgcolor="lightgray"> | ||
| Result: [{pod.result.count}]: | ||
| </Box> | ||
| <Box> | ||
| <Box component="pre" whiteSpace="pre-wrap"> | ||
| {pod.result.text} | ||
| </Box> |
There was a problem hiding this comment.
We need to show pod.result.text. In general, when a code block executes in Jupyter, the value of the last expression should be printed. like this:
There was a problem hiding this comment.
I was wondering on this too. At first I thought this was due to that CodePod does not return the evaluation of the last expression.
ui/src/components/Canvas.tsx
Outdated
| const onResize = useCallback((e, data) => { | ||
| const { size } = data; | ||
| const node = nodesMap.get(id); | ||
| if (node) { | ||
| node.style = { ...node.style, width: size.width }; | ||
| nodesMap.set(id, node); | ||
| updatePod({ | ||
| id, | ||
| data: { | ||
| width: size.width, | ||
| height: pod.height, | ||
| }, | ||
| }); | ||
| } | ||
| }, []); | ||
| const onLayout = useCallback(({ height }) => { | ||
| const node = nodesMap.get(id); | ||
| if (node) { | ||
| node.style = { ...node.style, height }; | ||
| nodesMap.set(id, node); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
These were removed in #61. So there are still merging problems. There might be other places, it's tedious to check. If it is not easy to merge, maybe let's create a clean PR from the current main branch?
There was a problem hiding this comment.
this is for width adjustment, it's different from removed one
| @@ -32,6 +32,7 @@ | |||
| "react-icons": "^4.6.0", | |||
| "react-monaco-editor": "^0.50.1", | |||
| "react-moveable": "^0.40.0", | |||
| "react-resizable": "^3.0.4", | |||
There was a problem hiding this comment.
Need this:
yarn add -D @types/react-resizable
Otherwise the compiler complains:
ERROR in src/components/Canvas.tsx:37:30
TS2307: Cannot find module 'react-resizable' or its corresponding type declarations.
35 |
36 | import Moveable from "react-moveable";
> 37 | import { ResizableBox } from "react-resizable";
| ^^^^^^^^^^^^^^^^^
38 | import Ansi from "ansi-to-react";
39 |
40 | import { customAlphabet } from "nanoid";
There was a problem hiding this comment.
I was wondering why...I shelled into the UI instance to manually install react-resizable to fix.
|
The merging problems seem complex. There is a lot of code that was removed in #61 that got added back in this PR. It's pretty hard to catch what they are. Also, this PR is getting too large to review. This might create additional conflicts with other changes that Xinyi and I are working on. Maybe let's split this PR into smaller pieces? |
Move the toolbar horizontally
add delete pod