fix: revert font-family escaping introduced by #4545 (#5164)

Using `CSS.escape` is the wrong tool for the job here:
 - it is meant for CSS selectors and does not handle CSS variables properly.
 - you can't use `var(--title)` as a font-family because it was getting escaped to `var\(--title\)`
This commit is contained in:
Nick Perez 2024-06-04 09:37:43 +02:00 committed by GitHub
parent 74bfdc5bef
commit f635d7b4f5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 82 additions and 3 deletions

View File

@ -1,3 +1,5 @@
import './styles.scss'
import Document from '@tiptap/extension-document' import Document from '@tiptap/extension-document'
import FontFamily from '@tiptap/extension-font-family' import FontFamily from '@tiptap/extension-font-family'
import Paragraph from '@tiptap/extension-paragraph' import Paragraph from '@tiptap/extension-paragraph'
@ -15,6 +17,7 @@ export default () => {
<p><span style="font-family: serif">Serious people use serif fonts anyway.</span></p> <p><span style="font-family: serif">Serious people use serif fonts anyway.</span></p>
<p><span style="font-family: monospace">The cool kids can apply monospace fonts aswell.</span></p> <p><span style="font-family: monospace">The cool kids can apply monospace fonts aswell.</span></p>
<p><span style="font-family: cursive">But hopefully we all can agree, that cursive fonts are the best.</span></p> <p><span style="font-family: cursive">But hopefully we all can agree, that cursive fonts are the best.</span></p>
<p><span style="font-family: var(--title-font-family)">Then there are CSS variables, the new hotness.</span></p>
`, `,
}) })
@ -27,6 +30,7 @@ export default () => {
<button <button
onClick={() => editor.chain().focus().setFontFamily('Inter').run()} onClick={() => editor.chain().focus().setFontFamily('Inter').run()}
className={editor.isActive('textStyle', { fontFamily: 'Inter' }) ? 'is-active' : ''} className={editor.isActive('textStyle', { fontFamily: 'Inter' }) ? 'is-active' : ''}
data-test-id="inter"
> >
Inter Inter
</button> </button>
@ -37,28 +41,46 @@ export default () => {
? 'is-active' ? 'is-active'
: '' : ''
} }
data-test-id="comic-sans"
> >
Comic Sans Comic Sans
</button> </button>
<button <button
onClick={() => editor.chain().focus().setFontFamily('serif').run()} onClick={() => editor.chain().focus().setFontFamily('serif').run()}
className={editor.isActive('textStyle', { fontFamily: 'serif' }) ? 'is-active' : ''} className={editor.isActive('textStyle', { fontFamily: 'serif' }) ? 'is-active' : ''}
data-test-id="serif"
> >
serif serif
</button> </button>
<button <button
onClick={() => editor.chain().focus().setFontFamily('monospace').run()} onClick={() => editor.chain().focus().setFontFamily('monospace').run()}
className={editor.isActive('textStyle', { fontFamily: 'monospace' }) ? 'is-active' : ''} className={editor.isActive('textStyle', { fontFamily: 'monospace' }) ? 'is-active' : ''}
data-test-id="monospace"
> >
monospace monospace
</button> </button>
<button <button
onClick={() => editor.chain().focus().setFontFamily('cursive').run()} onClick={() => editor.chain().focus().setFontFamily('cursive').run()}
className={editor.isActive('textStyle', { fontFamily: 'cursive' }) ? 'is-active' : ''} className={editor.isActive('textStyle', { fontFamily: 'cursive' }) ? 'is-active' : ''}
data-test-id="cursive"
> >
cursive cursive
</button> </button>
<button onClick={() => editor.chain().focus().unsetFontFamily().run()}> <button
onClick={() => editor.chain().focus().setFontFamily('var(--title-font-family)').run()}
className={editor.isActive('textStyle', { fontFamily: 'var(--title-font-family)' }) ? 'is-active' : ''}
data-test-id="css-variable"
>
CSS variable
</button>
<button
onClick={() => editor.chain().focus().setFontFamily('"Comic Sans MS", "Comic Sans"').run()}
className={editor.isActive('textStyle', { fontFamily: '"Comic Sans"' }) ? 'is-active' : ''}
data-test-id="comic-sans-quoted"
>
Comic Sans quoted
</button>
<button onClick={() => editor.chain().focus().unsetFontFamily().run()} data-test-id="unsetFontFamily">
unsetFontFamily unsetFontFamily
</button> </button>

View File

@ -3,5 +3,47 @@ context('/src/Extensions/FontFamily/React/', () => {
cy.visit('/src/Extensions/FontFamily/React/') cy.visit('/src/Extensions/FontFamily/React/')
}) })
// TODO: Write tests beforeEach(() => {
cy.get('.tiptap').then(([{ editor }]) => {
editor.commands.setContent('<p>Example Text</p>')
})
cy.get('.tiptap').type('{selectall}')
})
it('should set the font-family of the selected text', () => {
cy.get('[data-test-id="monospace"]')
.should('not.have.class', 'is-active')
.click()
.should('have.class', 'is-active')
cy.get('.tiptap').find('span').should('have.attr', 'style', 'font-family: monospace')
})
it('should remove the font-family of the selected text', () => {
cy.get('[data-test-id="monospace"]').click()
cy.get('.tiptap span').should('exist')
cy.get('[data-test-id="unsetFontFamily"]').click()
cy.get('.tiptap span').should('not.exist')
})
it('should work with font-family that have spaces in them', () => {
cy.get('[data-test-id="comic-sans"]')
.should('not.have.class', 'is-active')
.click()
.should('have.class', 'is-active')
cy.get('.tiptap').find('span').should('have.attr', 'style', 'font-family: Comic Sans MS, Comic Sans')
})
it('should allow CSS variables as a font-family', () => {
cy.get('[data-test-id="css-variable"]')
.should('not.have.class', 'is-active')
.click()
.should('have.class', 'is-active')
cy.get('.tiptap').find('span').should('have.attr', 'style', 'font-family: var(--title-font-family)')
})
}) })

View File

@ -0,0 +1,15 @@
html {
--title-font-family: 'Helvetica', sans-serif;
}
.tiptap {
> * + * {
margin-top: 0.75em;
}
ul,
ol {
padding: 0 1rem;
}
}

View File

@ -56,7 +56,7 @@ export const FontFamily = Extension.create<FontFamilyOptions>({
} }
return { return {
style: `font-family: ${attributes.fontFamily.split(',').map((fontFamily: string) => CSS.escape(fontFamily.trim())).join(', ')}`, style: `font-family: ${attributes.fontFamily}`,
} }
}, },
}, },