Skip to content
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

Document Editor page #56

Merged
merged 1 commit into from
Jul 24, 2024
Merged

Document Editor page #56

merged 1 commit into from
Jul 24, 2024

Conversation

csansoon
Copy link
Contributor

@csansoon csansoon commented Jul 23, 2024

First iteration of the document editor page. Clicking on a document from the sidebar shows the editor, which can be used to edit documents from draft commits.

Screen.Recording.2024-07-23.at.17.04.59.mov

@csansoon csansoon force-pushed the document-editor branch 4 times, most recently from 552ac7d to 9339d63 Compare July 23, 2024 13:57
Comment on lines 26 to 38
// When imported, Monaco automatically tries to use the window object.
// Since this is not available when rendering on the server, we only
// render the fallback component for SSR.
if (typeof window === 'undefined') return <DocumentTextEditorFallback />
const [isClient, setIsClient] = useState(false)

useEffect(() => {
setIsClient(typeof window !== 'undefined')
}, [])

if (!isClient) {
return <DocumentTextEditorFallback />
}
return <DocumentTextEditor {...props} />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the "hydration" error that Next.js shows when loading the editor, because it rendered something different in SSR and in the browser. Now, both render the fallback at first and then the browser updates to the actual editor (which renders the fallback too until monaco fully loads anyways)

@csansoon csansoon requested a review from andresgutgon July 23, 2024 13:59
@csansoon csansoon force-pushed the document-editor branch 7 times, most recently from 47cefff to f8c081f Compare July 24, 2024 08:49
const document = documents.find((d) => d.path === path)
if (!document) {
throw new NotFoundError('Document not found')
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a core method. originalGetDocumentByPath


const documentContent = documentsByPath[path]
if (documentContent === undefined) {
throw new Error('Document not found')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who handles this error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler does

onMount={handleEditorDidMount}
onChange={handleValueChange}
options={{
fixedOverflowWidgets: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So much pain in this boolean 😂

andresgutgon
andresgutgon previously approved these changes Jul 24, 2024
Base automatically changed from recompute-all-draft-changes-when-updating-any-document to main July 24, 2024 09:02
@csansoon csansoon dismissed andresgutgon’s stale review July 24, 2024 09:02

The base branch was changed.

@csansoon csansoon merged commit 7a75ecb into main Jul 24, 2024
2 checks passed
@csansoon csansoon deleted the document-editor branch July 24, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants