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

Editing markdown documentation with buttons #12217

Open
wants to merge 32 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
8d0baa0
fix vueHost type
vitvakatu Jan 27, 2025
488e3d3
Initial unit test for adding headers in markdown editor
vitvakatu Jan 28, 2025
f3b1b0e
Buttons for headers 1-3
vitvakatu Jan 28, 2025
b112950
Toggle headers
vitvakatu Feb 2, 2025
534cc0f
Headers in code blocks
vitvakatu Feb 2, 2025
4f24d18
Additional unit tests for headers
vitvakatu Feb 2, 2025
4bcab86
Markdown quotes
vitvakatu Feb 2, 2025
d0f958f
Basic unordered lists
vitvakatu Feb 3, 2025
2708b29
Ordered lists
vitvakatu Feb 3, 2025
87075cc
Refactorings for headers
vitvakatu Feb 3, 2025
a9d2eb8
moving files around
vitvakatu Feb 3, 2025
ac854f7
refactorings
vitvakatu Feb 3, 2025
ce970c4
Merge with develop
vitvakatu Feb 3, 2025
c22b0b2
Better headers editing in code blocks
vitvakatu Feb 4, 2025
93e2f68
Better handling of quotes edits in code blocks
vitvakatu Feb 4, 2025
4c839e3
Better editing of lists in code blocks
vitvakatu Feb 4, 2025
bab6f22
Add docs
vitvakatu Feb 4, 2025
7dae675
MutableChangeSet, refactorings
vitvakatu Feb 4, 2025
e3b96d5
refactoring: headers
vitvakatu Feb 4, 2025
ae963da
refactorings: quotes
vitvakatu Feb 4, 2025
86947e9
fix headers with inline styles
vitvakatu Feb 4, 2025
fb165cf
refactorings: lists
vitvakatu Feb 4, 2025
802a3f3
more refactorings
vitvakatu Feb 4, 2025
4f90de4
Merge with develop
vitvakatu Feb 4, 2025
10c8156
Update changelog
vitvakatu Feb 4, 2025
29fad32
Add tests for list marker lengths adjusting
vitvakatu Feb 5, 2025
7334941
remove vuehost from markdownEditing tests
vitvakatu Feb 6, 2025
309d297
fix unnecessary document recreation
vitvakatu Feb 6, 2025
a0b38d3
fix selection position in markdown editing tests
vitvakatu Feb 6, 2025
237f7a8
use dropdown menu for markdown editing buttons
vitvakatu Feb 6, 2025
c9bad7a
Use text3 icon
vitvakatu Feb 6, 2025
97d6940
isolate markdown editing in MarkdownEditorImpl
vitvakatu Feb 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
keys jumps to next cell/ next row respectively.][12129]
- [Fixed bugs occurring after renaming project from within graph editor][12106].
- [Added support for rendering numbered and nested lists][12190].
- [Added buttons for editing top-level markdown elements in the documentation
panel][12217].
- [Removed `#` from default colum name][12222]

[11889]: https://github.com/enso-org/enso/pull/11889
Expand All @@ -31,6 +33,7 @@
[12106]: https://github.com/enso-org/enso/pull/12106
[12190]: https://github.com/enso-org/enso/pull/12190
[12222]: https://github.com/enso-org/enso/pull/12222
[12217]: https://github.com/enso-org/enso/pull/12217

#### Enso Standard Library

Expand Down
14 changes: 14 additions & 0 deletions app/gui/src/project-view/components/DocumentationEditor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,20 @@ const handler = documentationEditorBindings.handler({
<div class="DocumentationEditor">
<div ref="toolbarElement" class="toolbar">
<FullscreenButton v-model="fullscreen" />
<SvgButton name="header1" title="Header 1" @click.stop="markdownEditor?.toggleHeader(1)" />
<SvgButton name="header2" title="Header 2" @click.stop="markdownEditor?.toggleHeader(2)" />
<SvgButton name="header3" title="Header 3" @click.stop="markdownEditor?.toggleHeader(3)" />
<SvgButton name="quote" title="Quote" @click.stop="markdownEditor?.toggleQuote()" />
<SvgButton
name="bullet-list"
title="Bullet list"
@click.stop="markdownEditor?.toggleList('unordered')"
/>
<SvgButton
name="numbered-list"
title="Numbered list"
@click.stop="markdownEditor?.toggleList('ordered')"
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

The block type should probably be a dropdown, top-level toggle buttons for everything would get unwieldy. We should be able to use the same menu code we used for Lexical:
https://github.com/enso-org/enso/pull/11469/files#diff-c9082ea1842a4e8a415bc841239cb0866d6d8ef678c957fe94b033e8cc8ef6d9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

<SvgButton name="image" title="Insert image" @click.stop="tryUploadImageFile()" />
</div>
<slot name="belowToolbar" />
Expand Down
10 changes: 10 additions & 0 deletions app/gui/src/project-view/components/MarkdownEditor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
provideDocumentationImageUrlTransformer,
type UrlTransformer,
} from '@/components/MarkdownEditor/imageUrlTransformer'
import { HeaderLevel, ListType } from '@/util/codemirror/markdownEditing'
import { Vec2 } from '@/util/data/vec2'
import { ComponentInstance, computed, defineAsyncComponent, ref, toRef } from 'vue'
import * as Y from 'yjs'
Expand All @@ -29,6 +30,15 @@ defineExpose({
putTextAtCoord: (text: string, coords: Vec2) => {
inner.value?.putTextAtCoords(text, coords)
},
toggleHeader: (level: HeaderLevel) => {
inner.value?.toggleHeader(level)
},
toggleQuote: () => {
inner.value?.toggleQuote()
},
toggleList: (type: ListType) => {
inner.value?.toggleList(type)
},
})
</script>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,20 @@ const editing = computed(() => !readonly.value && focused.value)

const vueHost = new VueHostInstance()
const editorRoot = useTemplateRef<ComponentInstance<typeof CodeMirrorRoot>>('editorRoot')
const { editorView, readonly, putTextAt } = useCodeMirror(editorRoot, {
content: () => content,
extensions: [
minimalSetup,
EditorView.lineWrapping,
highlightStyle(useCssModule()),
EditorView.clipboardInputFilter.of(transformPastedText),
ensoMarkdown(),
],
vueHost: () => vueHost,
})
const { editorView, readonly, putTextAt, toggleHeader, toggleQuote, toggleList } = useCodeMirror(
editorRoot,
{
content: () => content,
extensions: [
minimalSetup,
EditorView.lineWrapping,
highlightStyle(useCssModule()),
EditorView.clipboardInputFilter.of(transformPastedText),
ensoMarkdown(),
],
vueHost: () => vueHost,
},
)

useLinkTitles(editorView, { readonly })

Expand All @@ -55,6 +58,9 @@ defineExpose({
const pos = editorView.posAtCoords(coords, false)
putTextAt(text, pos, pos)
},
toggleHeader,
toggleQuote,
toggleList,
Copy link
Contributor

Choose a reason for hiding this comment

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

DocumentationEditor is the component that integrates the markdown editor with the graph editor. Pure markdown-editing functionality should be in the MarkdownEditorImpl, so that the dashboard can use it too (once they enable editing as planned).

We should render the toolbar in this component. Instead of exposing the formatting functions, we can use slots to inject the functionality that isn't shared between graph editor and dashboard (fullscreen, insert-image).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

})
</script>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,274 @@
import { ensoMarkdown } from '@/components/MarkdownEditor/markdown'
import {
HeaderLevel,
toggleHeader,
toggleList,
toggleQuote,
} from '@/util/codemirror/markdownEditing'
import { setVueHost } from '@/util/codemirror/vueHostExt'
import { EditorState } from '@codemirror/state'
import { EditorView } from '@codemirror/view'
import { expect, test } from 'vitest'

/**
* Setup editor with selection ranging from the first occurence of '|' in the `source` string to the last occurence of '|'.
* If there is a single '|', it points at the cursor position.
*/
const setupEditor = (source: string) => {
const selectionStart = source.indexOf('|')
const selectionEnd = source.lastIndexOf('|')
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be off by 1 because it is computed before the replaceAll.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, though it does not matter match in these tests. We are not too worried about tricky edge-cases, as even in the worse case edits are easily reversible even by hand.

const selection = { anchor: selectionStart, head: selectionEnd }
const doc = source.replaceAll('|', '')
const view = new EditorView({
state: EditorState.create({
doc,
extensions: ensoMarkdown(),
selection,
}),
})
const vueHost = {
register: () => ({
unregister: () => {},
update: () => {},
}),
teleportations: new Map(),
}
view.dispatch({ effects: setVueHost.of(vueHost) })
Copy link
Contributor

Choose a reason for hiding this comment

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

These tests shouldn't need a VueHost.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed it

return view
}

interface TestCase {
desc?: string
source: string
expected: string
}

interface HeaderTestCase extends TestCase {
headerLevel: HeaderLevel
}

const headerTestCases: HeaderTestCase[] = [
{
source: 'Some| text',
headerLevel: 1,
expected: '# Some text',
},
{
source: '**Bold| text**',
headerLevel: 1,
expected: '# **Bold text**',
},
{
source: '|Some| text',
headerLevel: 1,
expected: '# Some text',
},
{
source: '|Some| text',
headerLevel: 2,
expected: '## Some text',
},
{
source: '## |Some text',
headerLevel: 1,
expected: '# Some text',
},
{
source: '### |Some text',
headerLevel: 1,
expected: '# Some text',
},
{
source: 'Fir|st line\nSecond| line',
headerLevel: 1,
expected: '# First line\n# Second line',
},
{
source: '# Fir|st line\n# Second| line',
headerLevel: 1,
expected: 'First line\nSecond line',
},
{
source: '# |Header',
headerLevel: 1,
expected: 'Header',
},
{
source: '# **Bo|ld**',
headerLevel: 1,
expected: '**Bold**',
},
{
source: '# |Don’t touch this one\n## Touch this one\nMake this one h|eader',
headerLevel: 1,
expected: '# Don’t touch this one\n# Touch this one\n# Make this one header',
},
{
source: '```\nSome code\nHead|er in code block\nMore code\n```',
headerLevel: 1,
expected: '```\nSome code\n# Header in code block\nMore code\n```',
},
{
source: 'Some paragraph\n```\nSome code\n# Head|er in code block\nMore code\n```',
headerLevel: 2,
expected: 'Some paragraph\n```\nSome code\n## Header in code block\nMore code\n```',
},
{
source: '> This is a quote\nHeader| in quote',
headerLevel: 1,
expected: '> This is a quote\n# Header in quote',
},
{
source: '1. This is a list item\n2. This is| a future header',
headerLevel: 1,
expected: '1. This is a list item\n# 2. This is a future header',
},
]

test.each(headerTestCases)('markdown headers $source', ({ source, headerLevel, expected }) => {
const view = setupEditor(source)
toggleHeader(view, headerLevel)
expect(view.state.doc.toString()).toEqual(expected)
})

const quotesTestCases: TestCase[] = [
{
desc: 'Create simple quote',
source: 'This| is a quote',
expected: '> This is a quote',
},
{
desc: 'Multiline quote',
source: 'This |is a quote\nThis is anoth|er quote',
expected: '> This is a quote\nThis is another quote',
},
{
desc: 'Disable quote',
source: '> This |is a quote',
expected: 'This is a quote',
},
{
desc: 'Disable multiline quote',
source: '> This is| a quote\nThis is |another quote\n\nThis is a new paragraph',
expected: 'This is a quote\nThis is another quote\n\nThis is a new paragraph',
},
{
desc: 'Enable quote in code block',
source: '```\nSome code\nThis i|s a quote\nMore code\n```',
expected: '```\nSome code\n> This is a quote\nMore code\n```',
},
{
desc: 'Enable multiline quote in code block',
source: '```\nSome code\nThis i|s a quote\nAlso |a quote\nMore code\n```',
expected: '```\nSome code\n> This is a quote\nAlso a quote\nMore code\n```',
},
{
desc: 'Disable quote in code block',
source: '```\nSome code\n> This i|s a quote\nMore code\n```',
expected: '```\nSome code\nThis is a quote\nMore code\n```',
},
{
desc: 'Disable multiline quote in code block',
source: '```\nSome code\n> This i|s a quote\nAlso a q|uote\n\nMore code\n```',
expected: '```\nSome code\nThis is a quote\nAlso a quote\n\nMore code\n```',
},
]

test.each(quotesTestCases)('markdown quotes $desc', ({ source, expected }) => {
const view = setupEditor(source)
toggleQuote(view)
expect(view.state.doc.toString()).toEqual(expected)
})

const unorderedListTestCases: TestCase[] = [
{
desc: 'Create unordered list from empty line',
source: '|',
expected: '- ',
},
{
desc: 'Create simple unordered list',
source: '|List item\nList item\nList |item',
expected: '- List item\n- List item\n- List item',
},
{
desc: 'Disable unordered list',
source: '- Li|st item\n- List item\n- Lis|t item',
expected: 'List item\nList item\nList item',
},
{
desc: 'Change ordered list to unordered list',
source: '1. List| item\n2. List item\n3. Lis|t item',
expected: '- List item\n- List item\n- List item',
},
{
desc: 'Disable unordered list in code block',
source: '```\nSome code\n- Lis|t item\nMore code\n```',
expected: '```\nSome code\nList item\nMore code\n```',
},
{
desc: 'Create unordered list in code block',
source: '```\nSome code\nLis|t item\nAnother |list item\n```',
expected: '```\nSome code\n- List item\n- Another list item\n```',
},
{
desc: 'Change ordered list to unordered list in code block',
source: '```\nSome code\n1. List| item\n2. List item\n3. Lis|t item\nSome paragraph\n```',
expected: '```\nSome code\n- List item\n- List item\n- List item\nSome paragraph\n```',
},
{
desc: 'Disable unordered list in code block',
source: '```\nSome code\n- List| item\n- List item\n- Lis|t item\nSome paragraph\n```',
expected: '```\nSome code\nList item\nList item\nList item\nSome paragraph\n```',
},
]

test.each(unorderedListTestCases)('markdown unordered list $desc', ({ source, expected }) => {
const view = setupEditor(source)
toggleList(view, 'unordered')
expect(view.state.doc.toString()).toEqual(expected)
})

const orderedListTestCases: TestCase[] = [
{
desc: 'Create unordered list from empty line',
source: '|',
expected: '1. ',
},
{
desc: 'Create simple ordered list',
source: 'Li|st item\nList item\nLis|t item',
expected: '1. List item\n2. List item\n3. List item',
},
{
desc: 'Disable ordered list',
source: '1. Li|st item\n2. List item\n3. Lis|t item',
expected: 'List item\nList item\nList item',
},
{
desc: 'Change unordered list to ordered list',
source: '- List| item\n- List item\n- Lis|t item',
expected: '1. List item\n2. List item\n3. List item',
},
{
desc: 'Create ordered list in code block',
source: '```\nSome code\nLis|t item\nAnother |list item\n```',
expected: '```\nSome code\n1. List item\n2. Another list item\n```',
},
{
desc: 'Change unordered list to ordered list in code block',
source: '```\nSome code\n- List| item\n- List item\n- Lis|t item\nSome paragraph\n```',
expected: '```\nSome code\n1. List item\n2. List item\n3. List item\nSome paragraph\n```',
},
{
desc: 'Disable ordered list in code block',
source: '```\nSome code\n1. List| item\n2. List item\n3. Lis|t item\nSome paragraph\n```',
expected: '```\nSome code\nList item\nList item\nList item\nSome paragraph\n```',
},
]

test.each(orderedListTestCases)('markdown ordered list $desc', ({ source, expected }) => {
const view = setupEditor(source)
toggleList(view, 'ordered')
expect(view.state.doc.toString()).toEqual(expected)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any tests for checking what the "current" block type is identified as when different text is selected--that's complex and I think important to cover.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed, not needed. We might want to implement it in the future, but I don’t see much point given instant visual feedback in the editor, highlighting the block markers.

Loading
Loading