Skip to content

Commit

Permalink
fix: revert font-family escaping introduced by #4545 (#5164)
Browse files Browse the repository at this point in the history
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\)`
  • Loading branch information
nperez0111 authored Jun 4, 2024
1 parent 74bfdc5 commit f635d7b
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 3 deletions.
24 changes: 23 additions & 1 deletion demos/src/Extensions/FontFamily/React/index.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import './styles.scss'

import Document from '@tiptap/extension-document'
import FontFamily from '@tiptap/extension-font-family'
import Paragraph from '@tiptap/extension-paragraph'
Expand All @@ -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: 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: var(--title-font-family)">Then there are CSS variables, the new hotness.</span></p>
`,
})

Expand All @@ -27,6 +30,7 @@ export default () => {
<button
onClick={() => editor.chain().focus().setFontFamily('Inter').run()}
className={editor.isActive('textStyle', { fontFamily: 'Inter' }) ? 'is-active' : ''}
data-test-id="inter"
>
Inter
</button>
Expand All @@ -37,28 +41,46 @@ export default () => {
? 'is-active'
: ''
}
data-test-id="comic-sans"
>
Comic Sans
</button>
<button
onClick={() => editor.chain().focus().setFontFamily('serif').run()}
className={editor.isActive('textStyle', { fontFamily: 'serif' }) ? 'is-active' : ''}
data-test-id="serif"
>
serif
</button>
<button
onClick={() => editor.chain().focus().setFontFamily('monospace').run()}
className={editor.isActive('textStyle', { fontFamily: 'monospace' }) ? 'is-active' : ''}
data-test-id="monospace"
>
monospace
</button>
<button
onClick={() => editor.chain().focus().setFontFamily('cursive').run()}
className={editor.isActive('textStyle', { fontFamily: 'cursive' }) ? 'is-active' : ''}
data-test-id="cursive"
>
cursive
</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
</button>

Expand Down
44 changes: 43 additions & 1 deletion demos/src/Extensions/FontFamily/React/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,47 @@ context('/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)')
})
})
15 changes: 15 additions & 0 deletions demos/src/Extensions/FontFamily/React/styles.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

html {
--title-font-family: 'Helvetica', sans-serif;
}

.tiptap {
> * + * {
margin-top: 0.75em;
}

ul,
ol {
padding: 0 1rem;
}
}
2 changes: 1 addition & 1 deletion packages/extension-font-family/src/font-family.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export const FontFamily = Extension.create<FontFamilyOptions>({
}

return {
style: `font-family: ${attributes.fontFamily.split(',').map((fontFamily: string) => CSS.escape(fontFamily.trim())).join(', ')}`,
style: `font-family: ${attributes.fontFamily}`,
}
},
},
Expand Down

0 comments on commit f635d7b

Please sign in to comment.