Fix: some miscellaneous & tiny bugs #193

li-xin-yi · 2023-01-04T05:05:50Z

disable richNode editing & codeNode copying for guest users
fix the bug: a guest user sees the pasted pod increases on its size endlessly.
fix the bug: a pasted pod's position is not up-to-date when adding.
extend node types when observing adding nodes in nodesMap
refactor the code about the privilege management in front-end, hide the detailed role info to simplify the code

More details can be found in comments.

Suggestions:

Many functions in canvas.tsx can be moved into lib/utils.tsx like dbtype2nodetype to make canvas.tsx more clear.
For a guest user, the runtime should not be loaded from the beginning, it wastes resource.

Other works on the way:

Figure out a way to prevent selecting node by Space or Enter (see richtext pod moves as cursor moves in text #166)

li-xin-yi · 2023-01-04T05:37:51Z

Update: Fix the selection issue for guest users. Now they can also multi-select pods like other users. (pre-requirement for multi-pod copy by guests)

lihebi · 2023-01-04T05:41:24Z

ui/src/components/Canvas.tsx

-  const role = useStore(store, (state) => state.role);
+  const isGuest = useStore(store, (state) => state.isGuest());


Two reasons pull me back from this change.

Here, isGuest is the result of calling a Zustand action function state.isGuest(). I have never seen such usage before, and I doubt that it might be incompatible with zustand's ability to "Renders components only on changes". I.e., isGuest might not be able to trigger a re-render if store.role changes. Thus, I don't think we want to use this pattern.

Also, it adds two more actions (isGuest and isOwner) in Zustand store (which are effectively global functions). We want to keep Zustand store lean, only those states and state mutators.

For reference, below is Zustand store's typical usage pattern: state and state mutators:

const useFishStore = create((set) => ({ salmon: 1, tuna: 2, deleteEverything: () => set({}, true), // clears the entire store, actions included deleteTuna: () => set((state) => omit(state, ['tuna']), true), }))

Back to our problem, we could define isGuest and isOwner as functions, and use isGuest(role) here, e.g.,

// store/index.tsx or some utils.tsx export const isGuest = (role) => role === RoleType.GUEST // Code.tsx import {isGuest} from ... function Component() { const role = useStore(..., state.role) ... return <Box>{isGuest(role) && <Box>I'm guest</Box>}</Box> }

Actually I would remove enum RoleType and just use this (which is still type-safe):

// repoStateSlice.tsx export interface RepoStateSlice { ... role: "owner" | "guest" | "collaborator"; ... } // Code.tsx function Component() { const role = useStore(..., state.role) ... return <Box>{role === "guest" && <Box>I'm guest</Box>}</Box> }

I'm sure it must re-render when role changes because I set all the initial role as GUEST in the beginning, and it re-enables editing when the repo loads successfully for non-guest users. It's a function call evaluated in real-time, not a function declaration.

I just feel it may be problematic to expose the explicit role info to other components. We just need to know it abstractly. I have two ideas to do that:

Instead of setting role in store, split it to two fields isGuest: boolean and isOwner: boolean.

Similar to your solution, but hide the actual role like:

// store/index.tsx or some utils.tsx export const checkGuest = (role) => role === RoleType.GUEST // Code.tsx import {isGuest} from ... function Component() { const isGuest = useStore(..., checkGuest(state.role)) ... return <Box>{isGuest && <Box>I'm guest</Box>}</Box> }

Instead of setting role in store, split it to two fields isGuest: boolean and isOwner: boolean.

This will leave space for bugs, e.g. we can set both isGuest and isOwner to true.

Similar to your solution, but hide the actual role like:

Sounds good, let's use this.

li-xin-yi · 2023-01-04T08:56:41Z

Update: I finally removed the enum type and change it to role: "OWNER" | "COLLABORATOR" | "GUEST" (uppercase due to constant declaration).

There are still three known issues in this PR:

Auto-unselection on focusing codeNode/richNode doesn't work when the user is a guest. It seems to inactivate drag-handler settings of nodes when nodesDraggable=false, so no matter you click on any place inside the node, it will be selected immediately.
There exists some precision errors (or bias) when writing a float number into DB or in reverse. The node position coordinates become float numbers after dragging. So when you re-enter the repo after some drag operations, you may see some errors threw by verifyConsistency:

A possible solution: stop using !== to compare floats (many classic issues can be referenced in IEEE 754), you can use something like:

if (Math.abs(node1.position.x-node2.position.x) > Number.EPSILON) {
...
}

I set editable=false in Remirror component for guest users, the text cursor cannot be displayed in the node anymore, which means a guest cannot even select some texts. This is a totally different behavior from the official demo in storybook. But when you double-click on the text, the floating toolbar shows up, you can still bold, underscore, or highlight all texts. To solve the problem temporarily, you can implement the "read-only editor" manually by:

            onChange={(parameter) => {
              if(isGuest && parameter.tr?.docChanged) return;
              ...
            }}

But I feel many features are really wired in Remirror. If you have time, can you investigate it more and find any clue?

lihebi · 2023-01-04T17:35:24Z

3. the text cursor cannot be displayed in the node anymore, which means a guest cannot even select some texts.

It is because reactflow nodes have user-select: "none" CSS property. We just need to set it back:

<Box sx={{userSelect: "text", cursor: "auto"}}>
  ...
  <Remirror/>
  ...
</Box>

Let's fix it in another PR.

lihebi

Thank you, Xinyi!

lihebi · 2023-01-04T18:52:32Z

sx={{userSelect: "text", cursor: "auto"}}

For reference, PR #196 allows text selection in result block.

li-xin-yi added 3 commits January 4, 2023 04:52

tiny fix

bd4e738

remove infeasible setting

44deaba

fix selection for guests

3b86c48

lihebi reviewed Jan 4, 2023

View reviewed changes

li-xin-yi added 2 commits January 4, 2023 07:33

refactor role

9f7d423

typo

0d6ba18

lihebi approved these changes Jan 4, 2023

View reviewed changes

lihebi merged commit b118b80 into codepod-io:main Jan 4, 2023

li-xin-yi deleted the fix/misc branch January 5, 2023 00:25

li-xin-yi mentioned this pull request Jan 5, 2023

Minor fixes #199

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix: some miscellaneous & tiny bugs #193

Fix: some miscellaneous & tiny bugs #193

Uh oh!

li-xin-yi commented Jan 4, 2023 •

edited

Loading

Uh oh!

li-xin-yi commented Jan 4, 2023 •

edited

Loading

Uh oh!

lihebi Jan 4, 2023

Uh oh!

lihebi Jan 4, 2023 •

edited

Loading

Uh oh!

li-xin-yi Jan 4, 2023

Uh oh!

lihebi Jan 4, 2023

Uh oh!

li-xin-yi commented Jan 4, 2023 •

edited

Loading

Uh oh!

lihebi commented Jan 4, 2023 •

edited

Loading

Uh oh!

lihebi left a comment

Uh oh!

lihebi commented Jan 4, 2023

Uh oh!

Uh oh!

		const role = useStore(store, (state) => state.role);
		const isGuest = useStore(store, (state) => state.isGuest());

Fix: some miscellaneous & tiny bugs #193

Fix: some miscellaneous & tiny bugs #193

Uh oh!

Conversation

li-xin-yi commented Jan 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

li-xin-yi commented Jan 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lihebi Jan 4, 2023

Choose a reason for hiding this comment

Uh oh!

lihebi Jan 4, 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!

li-xin-yi Jan 4, 2023

Choose a reason for hiding this comment

Uh oh!

lihebi Jan 4, 2023

Choose a reason for hiding this comment

Uh oh!

li-xin-yi commented Jan 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lihebi commented Jan 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lihebi left a comment

Choose a reason for hiding this comment

Uh oh!

lihebi commented Jan 4, 2023

Uh oh!

Uh oh!

li-xin-yi commented Jan 4, 2023 •

edited

Loading

li-xin-yi commented Jan 4, 2023 •

edited

Loading

lihebi Jan 4, 2023 •

edited

Loading

li-xin-yi commented Jan 4, 2023 •

edited

Loading

lihebi commented Jan 4, 2023 •

edited

Loading