ronantakizawa/codereview-bench huggingface dataset:
Repository: ueberdosis/tiptap
File: packages/extension-link/src/helpers/clickHandler.ts (TypeScript)

Code before:
          let a = event.target as HTMLElement
          const els = []

          while (a.nodeName !== 'DIV') {
            els.push(a)
            a = a.parentNode as HTMLElement
          }
          link = els.find(value => value.nodeName === 'A') as HTMLAnchorElement
        }

        if (!link) {
          return false
        }

        const attrs = getAttributes(view.state, options.type.name)
        const href = link?.href ?? attrs.href
        const target = link?.target ?? attrs.target

        if (options.enableClickSelection) {
          const { doc, tr } = view.state
          const range = getMarkRange(doc.resolve(pos), options.type)
          if (range) {
            const { from, to } = range
            const $from = doc.resolve(from)
            const $to = doc.resolve(to)
            const transaction = tr.setSelection(new TextSelection($from, $to))
            view.dispatch(transaction)
          }
        }

        if (link && href) {
          window.open(href, target)

          return true
        }

        return false
      },
    },
  })
}


Review comment (suggestion):
I think you could use `extendMarkRange` instead to avoid handling the mark extension yourself

https://tiptap.dev/docs/editor/api/commands/nodes-and-marks/extend-mark-range

Code after:
          link = event.target
        } else {
          let a = event.target as HTMLElement
          const els = []

          while (a.nodeName !== 'DIV') {
            els.push(a)
            a = a.parentNode as HTMLElement
          }
          link = els.find(value => value.nodeName === 'A') as HTMLAnchorElement
        }

        if (!link) {
          return false
        }

        const attrs = getAttributes(view.state, options.type.name)
        const href = link?.href ?? attrs.href
        const target = link?.target ?? attrs.target

        if (options.enableClickSelection) {
          options.editor.commands.extendMarkRange(options.type.name)
        }

        if (link && href) {
          window.open(href, target)

          return true
        }

        return false
      },
    },
  })
}
Repository: ueberdosis/tiptap
File: demos/src/Extensions/CharacterCount/React/index.jsx (JavaScript)

Code before:
import './styles.scss'

import Document from '@tiptap/extension-document'
import Paragraph from '@tiptap/extension-paragraph'
import Text from '@tiptap/extension-text'
import { CharacterCount } from '@tiptap/extensions'
import { EditorContent, useEditor } from '@tiptap/react'
import React, { useState } from 'react'

const limit = 280

export default () => {
  const [charactersCount, setCharactersCount] = useState(0)
  const [wordsCount, setWordsCount] = useState(0)

  const handleUpdate = e => {
    setCharactersCount(e.storage.characterCount.characters())
    setWordsCount(e.storage.characterCount.words())
  }

  const editor = useEditor({
    extensions: [
      Document,
      Paragraph,
      Text,
      CharacterCount.configure({
        limit,
      }),
    ],
    content: `
        <p>
          Let‘s make sure people can’t write more than 280 characters. I bet you could build one of the biggest social networks on that idea.
        </p>
      `,
    onUpdate: event => handleUpdate(event.editor),
    onCreate: event => handleUpdate(event.editor),
  })


Review comment (refactor):
Instead of manually tracking character and word counts with ‎`useState` and update handlers, you could use the ‎`useEditorState` hook to access the current editor state and pull storage info directly. This would simplify the component and make it a bit more idiomatic for Tiptap React.

Example:

```ts
const { charactersCount, wordsCount } = useEditorState({
  editor,
  selector: (context) => ({
    charactersCount: context.editor.storage.characterCount.characters(),
    wordsCount: context.editor.storage.characterCount.words(),
  })
})
```

Code after:
import './styles.scss'

import Document from '@tiptap/extension-document'
import Paragraph from '@tiptap/extension-paragraph'
import Text from '@tiptap/extension-text'
import { CharacterCount } from '@tiptap/extensions'
import { EditorContent, useEditor, useEditorState } from '@tiptap/react'

const limit = 280

export default () => {
  const editor = useEditor({
    extensions: [
      Document,
      Paragraph,
      Text,
      CharacterCount.configure({
        limit,
      }),
    ],
    content: `
        <p>
          Let‘s make sure people can’t write more than 280 characters. I bet you could build one of the biggest social networks on that idea.
        </p>
      `,
  })

  const { charactersCount, wordsCount } = useEditorState({
    editor,
    selector: context => ({
      charactersCount: context.editor.storage.characterCount.characters(),
      wordsCount: context.editor.storage.characterCount.words(),
    }),
  })

  if (!editor) {
    return null
  }
Repository: ueberdosis/tiptap
File: packages/extension-horizontal-rule/src/horizontal-rule.ts (TypeScript)

Code before:
import { canInsertNode, isNodeSelection, mergeAttributes, Node, nodeInputRule } from '@tiptap/core'
import { NodeSelection, TextSelection } from '@tiptap/pm/state'

export interface HorizontalRuleOptions {
  /**
   * The HTML attributes for a horizontal rule node.
   * @default {}
   * @example { class: 'foo' }
   */
  HTMLAttributes: Record<string, any>,
  /**
   * The default type to insert after the horizontal rule.
   * @default "paragraph"
   * @example "heading"
   */
  defaultType: string,
}

declare module '@tiptap/core' {
  interface Commands<ReturnType> {
    horizontalRule: {
      /**
       * Add a horizontal rule
       * @example editor.commands.setHorizontalRule()
       */
      setHorizontalRule: () => ReturnType
    }
  }
}

/**
 * This extension allows you to insert horizontal rules.
 * @see https://www.tiptap.dev/api/nodes/horizontal-rule
 */
export const HorizontalRule = Node.create<HorizontalRuleOptions>({
  name: 'horizontalRule',

  addOptions() {
    return {
      HTMLAttributes: {},
      defaultType: "paragraph",

Review comment (suggestion):
I would rather call this `nextNodeType`.

```suggestion
  nextNodeType: string,
```

Code after:
import { canInsertNode, isNodeSelection, mergeAttributes, Node, nodeInputRule } from '@tiptap/core'
import { NodeSelection, TextSelection } from '@tiptap/pm/state'

export interface HorizontalRuleOptions {
  /**
   * The HTML attributes for a horizontal rule node.
   * @default {}
   * @example { class: 'foo' }
   */
  HTMLAttributes: Record<string, any>
  /**
   * The default type to insert after the horizontal rule.
   * @default "paragraph"
   * @example "heading"
   */
  nextNodeType: string
}

declare module '@tiptap/core' {
  interface Commands<ReturnType> {
    horizontalRule: {
      /**
       * Add a horizontal rule
       * @example editor.commands.setHorizontalRule()
       */
      setHorizontalRule: () => ReturnType
    }
  }
}

/**
 * This extension allows you to insert horizontal rules.
 * @see https://www.tiptap.dev/api/nodes/horizontal-rule
 */
export const HorizontalRule = Node.create<HorizontalRuleOptions>({
  name: 'horizontalRule',

  addOptions() {
    return {
      HTMLAttributes: {},
      nextNodeType: 'paragraph',
Repository: ueberdosis/tiptap
File: packages/extension-horizontal-rule/src/horizontal-rule.ts (TypeScript)

Code before:
  defaultType: string,
}

declare module '@tiptap/core' {
  interface Commands<ReturnType> {
    horizontalRule: {
      /**
       * Add a horizontal rule
       * @example editor.commands.setHorizontalRule()
       */
      setHorizontalRule: () => ReturnType
    }
  }
}

/**
 * This extension allows you to insert horizontal rules.
 * @see https://www.tiptap.dev/api/nodes/horizontal-rule
 */
export const HorizontalRule = Node.create<HorizontalRuleOptions>({
  name: 'horizontalRule',

  addOptions() {
    return {
      HTMLAttributes: {},
      defaultType: "paragraph",
    }
  },

  group: 'block',

  parseHTML() {
    return [{ tag: 'hr' }]
  },

  renderHTML({ HTMLAttributes }) {
    return ['hr', mergeAttributes(this.options.HTMLAttributes, HTMLAttributes)]
  },

  addCommands() {
    return {
      setHorizontalRule:
        () =>
        ({ chain, state }) => {
          // Check if we can insert the node at the current selection
          if (!canInsertNode(state, state.schema.nodes[this.name])) {
            return false
          }

          const { selection } = state
          const { $to: $originTo } = selection

Review comment (suggestion):
```suggestion
      nextNodeType: "paragraph",
```

Code after:
  nextNodeType: string
}

declare module '@tiptap/core' {
  interface Commands<ReturnType> {
    horizontalRule: {
      /**
       * Add a horizontal rule
       * @example editor.commands.setHorizontalRule()
       */
      setHorizontalRule: () => ReturnType
    }
  }
}

/**
 * This extension allows you to insert horizontal rules.
 * @see https://www.tiptap.dev/api/nodes/horizontal-rule
 */
export const HorizontalRule = Node.create<HorizontalRuleOptions>({
  name: 'horizontalRule',

  addOptions() {
    return {
      HTMLAttributes: {},
      nextNodeType: 'paragraph',
    }
  },

  group: 'block',

  parseHTML() {
    return [{ tag: 'hr' }]
  },

  renderHTML({ HTMLAttributes }) {
    return ['hr', mergeAttributes(this.options.HTMLAttributes, HTMLAttributes)]
  },

  addCommands() {
    return {
      setHorizontalRule:
        () =>
        ({ chain, state }) => {
          // Check if we can insert the node at the current selection
          if (!canInsertNode(state, state.schema.nodes[this.name])) {
            return false
          }

          const { selection } = state
          const { $to: $originTo } = selection
Repository: ueberdosis/tiptap
File: packages/extension-bubble-menu/src/bubble-menu-plugin.ts (TypeScript)

Code before:
    if (this.floatingUIOptions.inline) {
      middlewares.push(
        inline(typeof this.floatingUIOptions.inline !== 'boolean' ? this.floatingUIOptions.inline : undefined),
      )
    }

    return middlewares
  }

  constructor({
    editor,
    element,
    view,
    updateDelay = 250,
    resizeDelay = 60,
    shouldShow,
    appendTo,
    options,
  }: BubbleMenuViewProps) {
    this.editor = editor
    this.element = element
    this.view = view
    this.updateDelay = updateDelay
    this.resizeDelay = resizeDelay
    this.appendTo = appendTo
    this.scrollTarget = options?.scrollTarget ?? null

    this.floatingUIOptions = {
      ...this.floatingUIOptions,
      ...options,
    }

    this.element.tabIndex = 0

    if (shouldShow) {
      this.shouldShow = shouldShow
    }

    this.element.addEventListener('mousedown', this.mousedownHandler, { capture: true })
    this.view.dom.addEventListener('dragstart', this.dragstartHandler)
    this.editor.on('focus', this.focusHandler)
    this.editor.on('blur', this.blurHandler)
    window.addEventListener('resize', this.resizeHandler)
    ;(this.scrollTarget ?? window).addEventListener('scroll', this.resizeHandler)

    this.update(view, view.state)

    if (this.getShouldShow()) {
      this.show()
    }
  }

Review comment (refactor):
Possible refactor: 

```suggestion
    this.scrollTarget = options?.scrollTarget ?? window
```

Does it make sense or would it break things?

Code after:
    if (this.floatingUIOptions.inline) {
      middlewares.push(
        inline(typeof this.floatingUIOptions.inline !== 'boolean' ? this.floatingUIOptions.inline : undefined),
      )
    }

    return middlewares
  }

  constructor({
    editor,
    element,
    view,
    updateDelay = 250,
    resizeDelay = 60,
    shouldShow,
    appendTo,
    options,
  }: BubbleMenuViewProps) {
    this.editor = editor
    this.element = element
    this.view = view
    this.updateDelay = updateDelay
    this.resizeDelay = resizeDelay
    this.appendTo = appendTo
    this.scrollTarget = options?.scrollTarget ?? window

    this.floatingUIOptions = {
      ...this.floatingUIOptions,
      ...options,
    }

    this.element.tabIndex = 0

    if (shouldShow) {
      this.shouldShow = shouldShow
    }

    this.element.addEventListener('mousedown', this.mousedownHandler, { capture: true })
    this.view.dom.addEventListener('dragstart', this.dragstartHandler)
    this.editor.on('focus', this.focusHandler)
    this.editor.on('blur', this.blurHandler)
    window.addEventListener('resize', this.resizeHandler)
    this.scrollTarget.addEventListener('scroll', this.resizeHandler)

    this.update(view, view.state)

    if (this.getShouldShow()) {
      this.show()
    }
  }
Repository: ueberdosis/tiptap
File: packages/extension-table/src/cell/table-cell.ts (TypeScript)

Code before:
  addOptions() {
    return {
      HTMLAttributes: {},
    }
  },

  content: 'block+',

  addAttributes() {
    return {
      colspan: {
        default: 1,
      },
      rowspan: {
        default: 1,
      },
      colwidth: {
        default: null,
        parseHTML: element => {
          const colwidth = element.getAttribute('colwidth')
          const value = colwidth ? colwidth.split(',').map(width => parseInt(width, 10)) : null

          // if there is no colwidth attribute on the cell, try to get it from the colgroup
          if (!value) {
            const cols = element.closest('table')?.querySelectorAll('colgroup > col')
            const cellIndex = Array.from(element.parentElement?.children || []).indexOf(element)

            if (cols && cols[cellIndex]) {
              const colWidth = cols[cellIndex].getAttribute('width')
              return colWidth ? [parseInt(colWidth, 10)] : null
            }
          }

          return value
        },
      },
    }
  },

  tableRole: 'cell',

  isolating: true,

  parseHTML() {
    return [{ tag: 'td' }]
  },

  renderHTML({ HTMLAttributes }) {
    return ['td', mergeAttributes(this.options.HTMLAttributes, HTMLAttributes), 0]
  },
})

Review comment (suggestion):
Can it be the case that the cellIndex is null or -1? (for example, when the array is []). In the next line, you access `cols[cellIndex]` so I wonder what would happen if it is null.

Code after:
  addOptions() {
    return {
      HTMLAttributes: {},
    }
  },

  content: 'block+',

  addAttributes() {
    return {
      colspan: {
        default: 1,
      },
      rowspan: {
        default: 1,
      },
      colwidth: {
        default: null,
        parseHTML: element => {
          const colwidth = element.getAttribute('colwidth')
          const value = colwidth ? colwidth.split(',').map(width => parseInt(width, 10)) : null

          // if there is no colwidth attribute on the cell, try to get it from the colgroup
          if (!value) {
            const cols = element.closest('table')?.querySelectorAll('colgroup > col')
            const cellIndex = Array.from(element.parentElement?.children || []).indexOf(element)

            if (cellIndex && cellIndex > -1 && cols && cols[cellIndex]) {
              const colWidth = cols[cellIndex].getAttribute('width')
              return colWidth ? [parseInt(colWidth, 10)] : null
            }
          }

          return value
        },
      },
    }
  },

  tableRole: 'cell',

  isolating: true,

  parseHTML() {
    return [{ tag: 'td' }]
  },

  renderHTML({ HTMLAttributes }) {
    return ['td', mergeAttributes(this.options.HTMLAttributes, HTMLAttributes), 0]
  },
})
Repository: ueberdosis/tiptap
File: packages/extension-link/src/helpers/clickHandler.ts (TypeScript)

Code before:
type ClickHandlerOptions = {
  type: MarkType
  editor: Editor
  openOnClick?: boolean
  enableClickSelection?: boolean
}

export function clickHandler(options: ClickHandlerOptions): Plugin {
  return new Plugin({
    key: new PluginKey('handleClickLink'),
    props: {
      handleClick: (view, pos, event) => {
        if (event.button !== 0) {
          return false
        }

        if (!view.editable) {
          return false
        }

        let link: HTMLAnchorElement | null = null

        if (event.target instanceof HTMLAnchorElement) {
          link = event.target
        } else {
          link = (event.target as HTMLElement).closest('a')
        }

        if (!link) {
          return false
        }

        let handled = false

        if (options.enableClickSelection) {
          const commandResult = options.editor.commands.extendMarkRange(options.type.name)
          handled = commandResult
        }

        if (options.openOnClick) {
          const attrs = getAttributes(view.state, options.type.name)
          const href = link.href ?? attrs.href
          const target = link.target ?? attrs.target

          if (href) {
            window.open(href, target)
            handled = true
          }
        }

        return handled

Review comment (suggestion):
Just to make sure to not break out of the prosemirror div, I'd limit this lookup to the `editor.view.dom` element. Otherwise this is fine for me

Code after:
type ClickHandlerOptions = {
  type: MarkType
  editor: Editor
  openOnClick?: boolean
  enableClickSelection?: boolean
}

export function clickHandler(options: ClickHandlerOptions): Plugin {
  return new Plugin({
    key: new PluginKey('handleClickLink'),
    props: {
      handleClick: (view, pos, event) => {
        if (event.button !== 0) {
          return false
        }

        if (!view.editable) {
          return false
        }

        let link: HTMLAnchorElement | null = null

        if (event.target instanceof HTMLAnchorElement) {
          link = event.target
        } else {
          const target = event.target as HTMLElement | null
          if (!target) {
            return false
          }

          const root = options.editor.view.dom

          // Tntentionally limit the lookup to the editor root.
          // Using tag names like DIV as boundaries breaks with custom NodeViews,
          link = target.closest<HTMLAnchorElement>('a')

          if (link && !root.contains(link)) {
            link = null
          }
        }

        if (!link) {
          return false
        }

        let handled = false

        if (options.enableClickSelection) {
          const commandResult = options.editor.commands.extendMarkRange(options.type.name)
          handled = commandResult
        }
Repository: ueberdosis/tiptap
File: packages/extension-floating-menu/src/floating-menu-plugin.ts (TypeScript)

Code before:
      return
    }

    if (event?.relatedTarget && this.element.parentNode?.contains(event.relatedTarget as Node)) {
      return
    }

    if (event?.relatedTarget === this.editor.view.dom) {
      return
    }

    this.hide()
  }

  /**
   * Handles the transaction event to update the position of the floating menu.
   * This allows external code to trigger a position update via:
   * `editor.view.dispatch(editor.state.tr.setMeta('floatingMenu', 'updatePosition'))`
   */
  transactionHandler = ({ transaction: tr }: { transaction: Transaction }) => {
    const meta = tr.getMeta('floatingMenu')
    if (meta === 'updatePosition') {
      this.updatePosition()
    }
  }

  /**
   * Handles the window resize event to update the position of the floating menu.
   * It uses a debounce mechanism to prevent excessive updates.
   * The delay is defined by the `resizeDelay` property.
   */
  resizeHandler = () => {
    if (this.resizeDebounceTimer) {
      clearTimeout(this.resizeDebounceTimer)
    }

    this.resizeDebounceTimer = window.setTimeout(() => {
      this.updatePosition()
    }, this.resizeDelay)
  }

  updatePosition() {
    const { selection } = this.editor.state

    const domRect = posToDOMRect(this.view, selection.from, selection.to)

    const virtualElement = {
      getBoundingClientRect: () => domRect,
      getClientRects: () => [domRect],
    }


Review comment (suggestion):
Could the update be done with a specific Editor command, or would it not be possible?
Example: `editor.commands.updateFloatingMenuPosition()`

Code after:
    // we use `setTimeout` to make sure `selection` is already updated
    setTimeout(() => this.update(this.editor.view))
  }

  blurHandler = ({ event }: { event: FocusEvent }) => {
    if (this.preventHide) {
      this.preventHide = false

      return
    }

    if (event?.relatedTarget && this.element.parentNode?.contains(event.relatedTarget as Node)) {
      return
    }

    if (event?.relatedTarget === this.editor.view.dom) {
      return
    }

    this.hide()
  }

  /**
   * Handles the transaction event to update the position of the floating menu.
   * This allows external code to trigger a position update via:
   * `editor.view.dispatch(editor.state.tr.setMeta('floatingMenu', 'updatePosition'))`
   */
  transactionHandler = ({ transaction: tr }: { transaction: Transaction }) => {
    const meta = tr.getMeta('floatingMenu')
    if (meta === 'updatePosition') {
      this.updatePosition()
    }
  }

  /**
   * Handles the window resize event to update the position of the floating menu.
   * It uses a debounce mechanism to prevent excessive updates.
   * The delay is defined by the `resizeDelay` property.
   */
  resizeHandler = () => {
    if (this.resizeDebounceTimer) {
      clearTimeout(this.resizeDebounceTimer)
    }

    this.resizeDebounceTimer = window.setTimeout(() => {
      this.updatePosition()
    }, this.resizeDelay)
  }

  updatePosition() {
    const { selection } = this.editor.state
Repository: unoplatform/uno
File: src/Uno.UI/UI/Xaml/Shapes/Rectangle.cs (C#)

Code before:
﻿using Windows.Foundation;

namespace Windows.UI.Xaml.Shapes
{
	public partial class Rectangle : Shape
	{
		static Rectangle()
		{
			StretchProperty.OverrideMetadata(typeof(Rectangle), new FrameworkPropertyMetadata(defaultValue: Media.Stretch.Fill));
		}

#if __IOS__ || __MACOS__ || __SKIA__ || __ANDROID__ || __WASM__
		/// <inheritdoc />
		protected override Size MeasureOverride(Size availableSize)
			=> MeasureRelativeShape(availableSize);
#endif

		#region RadiusY (DP)
		public static DependencyProperty RadiusYProperty { get; } = DependencyProperty.Register(
			"RadiusY",
			typeof(double),
			typeof(Rectangle),
			new FrameworkPropertyMetadata(
				defaultValue: 0.0,
				options: FrameworkPropertyMetadataOptions.AffectsMeasure
			)
		);

		public double RadiusY
		{
			get => (double)this.GetValue(RadiusYProperty);
			set => this.SetValue(RadiusYProperty, value);
		}
		#endregion

		#region RadiusX (DP)
		public static DependencyProperty RadiusXProperty { get; } = DependencyProperty.Register(
			"RadiusX",
			typeof(double),
			typeof(Rectangle),
			new FrameworkPropertyMetadata(
				defaultValue: 0.0,
				options: FrameworkPropertyMetadataOptions.AffectsMeasure
			)
		);

Review comment (suggestion):
Hmm.. I don't think this should be affecting measure. But if that's what WinUI is doing let's follow that. I wouldn't expect this property to be changing dynamically that much in real scenarios, so doing the potentially unnecessary measure shouldn't be a problem.

Code after:
﻿using Windows.Foundation;

namespace Microsoft.UI.Xaml.Shapes
{
	public partial class Rectangle : Shape
	{
#if __IOS__ || __MACOS__ || __SKIA__ || __ANDROID__ || __WASM__
		/// <inheritdoc />
		protected override Size MeasureOverride(Size availableSize)
			=> MeasureRelativeShape(availableSize);
#endif

		#region RadiusY (DP)
		public static DependencyProperty RadiusYProperty { get; } = DependencyProperty.Register(
			"RadiusY",
			typeof(double),
			typeof(Rectangle),
			new FrameworkPropertyMetadata(
				defaultValue: 0.0,
				options: FrameworkPropertyMetadataOptions.AffectsMeasure
			)
		);

		public double RadiusY
		{
			get => (double)this.GetValue(RadiusYProperty);
			set => this.SetValue(RadiusYProperty, value);
		}
		#endregion

		#region RadiusX (DP)
		public static DependencyProperty RadiusXProperty { get; } = DependencyProperty.Register(
			"RadiusX",
			typeof(double),
			typeof(Rectangle),
			new FrameworkPropertyMetadata(
				defaultValue: 0.0,
				options: FrameworkPropertyMetadataOptions.AffectsMeasure
			)
		);

		public double RadiusX
		{
			get => (double)this.GetValue(RadiusXProperty);
			set => this.SetValue(RadiusXProperty, value);