-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: integrate sync eng #2924
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: integrate sync eng #2924
Changes from 145 commits
9667f2b
5bdf732
a1e9a3f
ad812f8
c15304a
c9cc777
7967d5f
5cd47ae
e6669c8
fdd56dc
8287347
71f4683
22c3783
c44beaf
282cc83
25726e6
07248c3
b569aeb
796a5bc
a89b5e5
bc6da3c
2dcc8ee
aae7228
9f9d70c
79d0b79
1669910
11b50d2
3a4eb51
a8364cc
e45e941
898d104
2c9c76e
bd882ac
a869bb9
3794efb
a11e57d
d519aba
e2509c8
c9e2294
2ce9005
87ef4a9
30c9fdb
f759b3e
7cdb8f7
8f8503e
f39e645
eb3b63d
b5e2276
4edd7b6
2791932
b98f10d
acfc83c
adbc0d8
7bc2cc0
3269ac3
245fd5e
7a965ab
676a005
3719653
114e496
bcd9197
4c610d8
94033f2
f2cd7da
0d770c7
9f5016e
10a7c79
17e0a1f
b4f0fc9
12a4720
5661778
ed62540
14cebae
10d5b76
3816d2f
0d50998
c4ad1b4
52c0f85
ae9996a
79e4ff0
ad4efa9
c5aee59
5b6d2b6
1755fce
97f5b22
ee0690a
31bad4b
ebfff2c
46c5211
a435fb0
656d134
905b611
dfce627
7a5620b
7bf1fbf
8716eb4
61601a6
cc30779
8acb3fa
b7e7426
7beb5ad
fb270ac
ba406a3
0d91716
4105a34
505a6c6
52b4c77
03879ef
13eb3af
b04152e
b0ba790
b46bba8
c8ddcb7
680c59c
b83e7e6
d92dae5
e520ad8
50f1e55
abb4e4c
b839148
1323252
1726923
dd32ecd
9c8c941
120bb4e
cb6e5a6
bf0da9e
4efd3dc
91debaa
59d6053
cc89adc
3053de1
13e6d0a
c9b1c57
098e25d
5d106d8
1aa23e8
032bb8d
336dcc7
a4fc99f
f67c75b
e38d38f
d1034cb
c220663
355299f
b11b001
29a3ba8
c2a0f36
5012335
6148c01
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,11 +7,11 @@ import { DropdownMenu, DropdownMenuContent, DropdownMenuTrigger } from '@onlook/ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Icons } from '@onlook/ui/icons'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { toNormalCase } from '@onlook/utility'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { observer } from 'mobx-react-lite'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useState } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useEffect, useState } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useDropdownControl } from '../../hooks/use-dropdown-manager'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { ToolbarButton } from '../../toolbar-button'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useTextControl } from '../../hooks/use-text-control'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { HoverOnlyTooltip } from '../../hover-tooltip'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { ToolbarButton } from '../../toolbar-button'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { FontFamily } from './font-family'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const FontFamilySelector = observer(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -26,6 +26,14 @@ export const FontFamilySelector = observer(() => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| font.family.toLowerCase().includes(search.toLowerCase()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TODO: use file system like code tab | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!editorEngine.activeSandbox.session.provider) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| editorEngine.font.init(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [editorEngine.activeSandbox.session.provider]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+30
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid MobX observables in useEffect dependency arrays. The dependency array includes
Consider using MobX's As per coding guidelines. Example refactor using +import { reaction } from 'mobx';
+import { useEffect, useState } from 'react';
-import { useEffect, useState } from 'react';
export const FontFamilySelector = observer(() => {
const editorEngine = useEditorEngine();
// ... other code ...
useEffect(() => {
- if (!editorEngine.activeSandbox.session.provider) {
- return;
- }
- editorEngine.font.init();
- }, [editorEngine.activeSandbox.session.provider]);
+ const dispose = reaction(
+ () => editorEngine.activeSandbox.session.provider,
+ (provider) => {
+ if (provider) {
+ editorEngine.font.init();
+ }
+ },
+ { fireImmediately: true }
+ );
+ return () => dispose();
+ }, [editorEngine]);📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleClose = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onOpenChange(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| editorEngine.state.brandTab = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Simplify root path handling now that
pathsEqualnormalizes paths.Line 55's conversion of
'/'to''is now redundant becausepathsEqualreturnsfalsefor empty strings. This means the first comparison in line 56 always fails for root paths, leaving only the second comparison to handle that case. The logic still works, but can be simplified.Apply this diff to remove the unnecessary conversion:
Or more concisely:
📝 Committable suggestion
🤖 Prompt for AI Agents