Skip to content

Commit

Permalink
fix: revert font-family escaping introduced by #4545
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\)`

Instead this introduces using a regex to see if we should quote the font-family. Quoting when:
 - font-family includes at least one number or whitespace character
  • Loading branch information
nperez0111 committed May 17, 2024
1 parent f55171f commit b84c62f
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 3 deletions.
17 changes: 16 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,39 @@ 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().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 quote 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;
}
}
3 changes: 2 additions & 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,8 @@ 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.split(',')
.map((fontFamily: string) => (fontFamily.match(/(\d|\s)+/g) ? `"${fontFamily.trim()}"` : fontFamily.trim())).join(', ')}`,
}
},
},
Expand Down

0 comments on commit b84c62f

Please sign in to comment.