mui-tiptap icon indicating copy to clipboard operation
mui-tiptap copied to clipboard

When Heading 1 is selected, all other heading levels are disabled

Open Kayeeec opened this issue 1 year ago • 4 comments

Describe the bug

If I style some text as Heading 1, I cannot switch it to any other heading level, because they are disabled in the MenuSelectHeading component. I have to first switch it to Paragraph and then to the desired Heading 2-6.

This only happens with Heading 1, all other heading levels are fine.

To Reproduce

I'll try to replicate it in codesandbox when I have the time.

Steps to reproduce the behaviour:

  1. write some text and style it as Heading 1
  2. with that text selected or having the cursor in it, open the MenuSelectHeading dropdown

Expected behaviour

All heading levels in the dropdown should be enabled and selectable.

E.g. it should be possible to select the option Heading 2, which should switch the selected/focused text to the Heading 2 style.

Actual behaviour

All heading levels are disabled and cannot be selected. You have to first select Paragraph, and then the desired Heading 2-6.

Screenshots

image

System (please complete the following information):

  • mui-tiptap version: ^1.8.6
  • tiptap version: ^2.2.3
  • Browser: Edge
  • Node version: 20.11.0
  • OS: Pop!_OS 22.04
  • "react": "^18.2.0"
  • "typescript": "^4.5.4",

Additional context

I compared this behaviour with your Codesandbox example, and there it works fine - you can switch heading levels directly. I tried replicating the codesandbox code as closely as possible in my project - using the same versions of the tiptap and mui-tiptap packages, using the HeadingWithAnchor instead of the basic Heading extension, same order of the extensions. Nothing helped. The older package versions caused weird internal "cannot access property of undefined" errors.

Code excerpts:

Editor definition:
const editor = useEditor({
  editorProps: {
    attributes: {
      'data-ui-input': 'rich-text-editor-input-field',
    },
  },
  extensions: [
    BulletList,
    CodeBlock,
    Document,
    HardBreak,
    ListItem,
    OrderedList,
    Paragraph,
    CustomSubscript,
    CustomSuperscript,
    Text,
    Bold,
    Blockquote,
    Code,
    Italic,
    Underline,
    Strike,
    CustomLinkExtension.configure({
      autolink: true,
      linkOnPaste: true,
      openOnClick: false,
    }),
    LinkBubbleMenuHandler,
    Gapcursor,
    Heading,
    HorizontalRule,
    Dropcursor,
    TaskList,
    TaskItem.configure({
      nested: true,
    }),
    History,
    Placeholder.configure({
      emptyEditorClass: styles.isEditorEmpty,
      placeholder,
    }),
    CharacterCount.configure({
      limit: maxLength,
    }),
  ],
  content,
}, [placeholder, content])
Component:
<ThemeProvider theme={muiTheme}>
  <RichTextEditorProvider editor={editor}>
    <RichTextField
      className={styles.editor}
      RichTextContentProps={{
        className: clsx(styles.richTextField, richTextFieldClassName),
      }}
      controls={(
        <MenuControlsContainer>
          <MenuButtonUndo
            data-ui-element="rich-text-menu-item-undo"
            tooltipLabel={t('richTextEditor.undo')}
          />
          <MenuButtonRedo
            data-ui-element="rich-text-menu-item-redo"
            tooltipLabel={t('richTextEditor.redo')}
          />
          <MenuDivider />
          <MenuSelectHeading
            data-ui-element="rich-text-menu-item-select-heading"
            className={styles.menuSelectHeading}
            tooltipTitle={t('richTextEditor.selectHeadingTooltip')}
            labels={{
              paragraph: t('richTextEditor.paragraph'),
              heading1: t('richTextEditor.heading1'),
              heading2: t('richTextEditor.heading2'),
              heading3: t('richTextEditor.heading3'),
              heading4: t('richTextEditor.heading4'),
              heading5: t('richTextEditor.heading5'),
              heading6: t('richTextEditor.heading6'),
            }}
          />
          <MenuDivider />
          <MenuButtonBold
            data-ui-element="rich-text-menu-item-bold"
            tooltipLabel={t('richTextEditor.bold')}
          />
          <MenuButtonItalic
            data-ui-element="rich-text-menu-item-italic"
            tooltipLabel={t('richTextEditor.italic')}
          />
          <MenuButtonUnderline
            data-ui-element="rich-text-menu-item-underline"
            tooltipLabel={t('richTextEditor.underline')}
          />
          <MenuButtonStrikethrough
            data-ui-element="rich-text-menu-item-strikethrough"
            tooltipLabel={t('richTextEditor.strikethrough')}
          />
          <MenuButtonSubscript
            data-ui-element="rich-text-menu-item-subscript"
            tooltipLabel={t('richTextEditor.subscript')}
          />
          <MenuButtonSuperscript
            data-ui-element="rich-text-menu-item-superscript"
            tooltipLabel={t('richTextEditor.superscript')}
          />
          <MenuDivider />
          <MenuButtonEditLink
            data-ui-element="rich-text-menu-item-edit-link"
            tooltipLabel={t('richTextEditor.link')}
          />
          <MenuDivider />
          <MenuButtonOrderedList
            data-ui-element="rich-text-menu-item-ordered-list"
            tooltipLabel={t('richTextEditor.orderedList')}
          />
          <MenuButtonBulletedList
            data-ui-element="rich-text-menu-item-bullet-list"
            tooltipLabel={t('richTextEditor.bulletedList')}
          />
          <MenuButtonTaskList
            data-ui-element="rich-text-menu-item-task-list"
            tooltipLabel={t('richTextEditor.taskChecklist')}
          />
          <MenuDivider />
          <MenuButtonBlockquote
            data-ui-element="rich-text-menu-item-blockquote"
            tooltipLabel={t('richTextEditor.blockquote')}
          />
          <MenuDivider />
          <MenuButtonCode
            data-ui-element="rich-text-menu-item-code"
            tooltipLabel={t('richTextEditor.code')}
          />
          <MenuButtonCodeBlock
            data-ui-element="rich-text-menu-item-code-block"
            tooltipLabel={t('richTextEditor.codeBlock')}
          />
          <MenuDivider />
          <MenuButtonHorizontalRule
            data-ui-element="rich-text-menu-item-horizontal-rule"
            tooltipLabel={t('richTextEditor.insertHorizontalLine')}
          />
          <MenuButtonRemoveFormatting
            data-ui-element="rich-text-menu-item-remove-formatting"
            tooltipLabel={t('richTextEditor.removeInlineFormatting')}
          />
        </MenuControlsContainer>
      )}
    />
    <LinkBubbleMenu
      labels={{
        viewLinkEditButtonLabel: t('edit'),
        viewLinkRemoveButtonLabel: t('remove'),
        editLinkAddTitle: t('richTextEditor.addLink'),
        editLinkEditTitle: t('richTextEditor.editLink'),
        editLinkCancelButtonLabel: t('cancel'),
        editLinkTextInputLabel: t('text'),
        editLinkHrefInputLabel: t('richTextEditor.link'),
        editLinkSaveButtonLabel: t('save'),
      }}
    />
  </RichTextEditorProvider>
</ThemeProvider>

Kayeeec avatar Feb 21 '24 13:02 Kayeeec

Hm, that's a strange bug! I was actually able to reproduce it with (1) your set of extensions and (2) the latest version of tiptap for all extensions. If either of those weren't done, it would work just fine (no bug). I haven't had a chance to look thoroughly into why that bug occurs if (1) and (2) are done, but I suspect it's an issue in Tiptap's can() function as described in this Tiptap issue (reportedly affecting Tiptap > 2.0.4): https://github.com/ueberdosis/tiptap/issues/4225#issuecomment-1682657548

since we call can().setHeading({ level: 1 }) in the MenuSelectHeading component to determine whether to allow the user to choose the heading option: https://github.com/sjdemartini/mui-tiptap/blob/b5f6d0907fde75b494aed97836ef12a02206513f/src/controls/MenuSelectHeading.tsx#L187-L188

It's seemed to me that newer Tiptap releases have been less stable and reliable (with occasional somewhat dangerous lower-level changes/bugs unfortunately), so your best bet may be to switch back to 2.0.* for now sadly, if that's an option. I have been swamped so don't suspect I'll have time to investigate a workaround fix for this underlying Tiptap problem in the near future, but would welcome further insight from you and/or PRs/fixes if you know of one.

sjdemartini avatar Feb 22 '24 01:02 sjdemartini

I'm not sure it's a bug on the TipTap side. See this codesandbox I managed to whip up (ignore the code quality, it was a quick job :sweat_smile: ):

https://codesandbox.io/p/devbox/zwn843

I used only tiptap ^2.2.3 so without mui-tiptap components. I modified a very simple toolbar from their example, and the editor?.can().setHeading(...) method seems to be working just fine. It does not disable the other heading buttons.

Kayeeec avatar Feb 22 '24 17:02 Kayeeec

I think I got it! The problem is that the level is always set to 1 when determining the canSetHeading.

Once a text is set to Heading 1, it cannot be set to Heading 1 again (it can be "toggled" via toggleHeading(...) but not "set" via setHeading(...)). Therefore editor?.can().setHeading({ level: 1 }) will return false. If it's set to any other Heading 2-6, it can be set to Heading 1, therefore editor?.can().setHeading({ level: 1 }) will return true. It explains the behaviour I described in the repro steps.

So I think the canSetHeading needs to be rewritten into a method that takes the actual level. If you think I'm correct, I could take a look at it and submit a PR.

Kayeeec avatar Feb 22 '24 22:02 Kayeeec

Thanks for investigating! Yes, I agree that that seems to be the cause of the issue on the newer version of Tiptap. It's a change in the longstanding behavior of can().setHeading(), and it seems Tiptap's release notes sadly didn't document this change. (The Heading extension file itself has not had any changes, so seems to be due to some lower level update.)

The code I linked to above from mui-tiptap is calling can().setHeading() only once as a performance optimization, rather than calling it for each heading level, since the "is user allowed to change to a heading" logic is the same for any given level. Despite Tiptap's new underlying behavior, we shouldn't disable the current heading's option in the dropdown options even if it's currently selected. For usability/accessibility/consistency with patterns seen across user interfaces, <select> elements don't mark the currently selected <option> as disabled (even if reselecting it would have no effect). i.e., if you are currently on some Heading 1 text, none the options in the dropdown should be disabled (like in the current CodeSandbox demo from the mui-tiptap README).

So perhaps the logic would need to be something like: canSetHeading = currentLevel === 1 || !!editor?.can().setHeading({ level: 1 }). Definitely open to reviewing a PR since I likely won't have time to work on this myself!

sjdemartini avatar Feb 24 '24 18:02 sjdemartini

Fixed in https://github.com/sjdemartini/mui-tiptap/pull/222 and released in https://github.com/sjdemartini/mui-tiptap/releases/tag/v1.9.0

sjdemartini avatar May 22 '24 18:05 sjdemartini