-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add TableCellImproved, TableHeaderImproved extensions and MenuSelectTableBorderStyle control #185
Changes from all commits
fecda46
9065a4a
f493ffd
a08d0ad
24e7a68
a636ccc
ee36e70
f7563d6
e267efe
e295a52
4ba6af4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,6 +87,7 @@ export type ControlledBubbleMenuProps = { | |
* content. | ||
*/ | ||
PaperProps?: Partial<PaperProps>; | ||
onMouseLeave?: (event: React.MouseEvent<HTMLElement>) => void; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this new prop is no longer needed? Not seeing it referenced elsewhere |
||
}; | ||
|
||
const controlledBubbleMenuClasses: ControlledBubbleMenuClasses = | ||
|
@@ -141,6 +142,7 @@ export default function ControlledBubbleMenu({ | |
], | ||
flipPadding = 8, | ||
PaperProps, | ||
onMouseLeave, | ||
}: ControlledBubbleMenuProps) { | ||
const { classes, cx } = useStyles(undefined, { | ||
props: { classes: overrideClasses }, | ||
|
@@ -222,6 +224,7 @@ export default function ControlledBubbleMenu({ | |
container={container} | ||
disablePortal={disablePortal} | ||
transition | ||
onMouseLeave={onMouseLeave} | ||
> | ||
{({ TransitionProps }) => ( | ||
<Fade | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import { findParentNodeClosestToPos, posToDOMRect } from "@tiptap/core"; | ||
import { findParentNodeOfType } from "prosemirror-utils"; | ||
import { useMemo } from "react"; | ||
import { makeStyles } from "tss-react/mui"; | ||
import type { Except } from "type-fest"; | ||
|
@@ -9,7 +10,6 @@ import { useRichTextEditorContext } from "./context"; | |
import TableMenuControls, { | ||
type TableMenuControlsProps, | ||
} from "./controls/TableMenuControls"; | ||
import { useDebouncedFocus } from "./hooks"; | ||
import DebounceRender, { | ||
type DebounceRenderProps, | ||
} from "./utils/DebounceRender"; | ||
|
@@ -70,15 +70,6 @@ export default function TableBubbleMenu({ | |
const editor = useRichTextEditorContext(); | ||
const { classes } = useStyles(); | ||
|
||
// Because the user interactions with the table menu bar buttons unfocus the | ||
// editor (since it's not part of the editor content), we'll debounce our | ||
// visual focused state so that we keep the bubble menu open during those | ||
// interactions. That way we don't close it upon menu bar button click | ||
// immediately, which can prevent menu button callbacks from working and | ||
// also undesirably will close the bubble menu rather than keeping it open for | ||
// future menu interaction. | ||
const isEditorFocusedDebounced = useDebouncedFocus({ editor }); | ||
Comment on lines
-73
to
-80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removing this condition does make it easier to ensure the "nested" menu logic works if editor focus inadvertently gets removed, but this has the downside that even if a user clicks out of the editor and is no longer interacting with it, the bubble menu stays open. Perhaps that's a worthwhile tradeoff. (as mentioned in #185 (comment)) |
||
|
||
// We want to position the table menu outside the entire table, rather than at the | ||
// current cursor position, so that it's essentially static even as the table changes | ||
// in size and doesn't "block" things within the table while you're trying to edit. | ||
|
@@ -143,10 +134,16 @@ export default function TableBubbleMenu({ | |
<TableMenuControls className={classes.controls} labels={labels} /> | ||
); | ||
|
||
const tableNode = findParentNodeOfType(editor.state.schema.nodes.table)( | ||
editor.state.selection | ||
); | ||
|
||
const shouldOpen = Boolean(tableNode); | ||
Comment on lines
+137
to
+141
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain the rationale behind this vs I imagine Tiptap may have a util equivalent to |
||
|
||
return ( | ||
<ControlledBubbleMenu | ||
editor={editor} | ||
open={isEditorFocusedDebounced && editor.isActive("table")} | ||
open={shouldOpen} | ||
anchorEl={bubbleMenuAnchorEl} | ||
// So the menu doesn't move as columns are added, removed, or resized, we | ||
// prefer "foo-start" rather than the centered "foo" placement. Similarly, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ export interface MenuButtonProps | |
* included. | ||
*/ | ||
tooltipLabel: MenuButtonTooltipProps["label"]; | ||
tooltipPlacement?: MenuButtonTooltipProps["placement"]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a doc comment like we have for the other props |
||
/** | ||
* (Optional) An array of the keyboard shortcut keys that trigger this action | ||
* will be displayed in a tooltip when hovering. If empty, no keyboard | ||
|
@@ -70,6 +71,7 @@ const useStyles = makeStyles({ name: { MenuButton } })({ | |
*/ | ||
export default function MenuButton({ | ||
tooltipLabel, | ||
tooltipPlacement, | ||
tooltipShortcutKeys, | ||
IconComponent, | ||
buttonRef, | ||
|
@@ -81,6 +83,7 @@ export default function MenuButton({ | |
<span className={classes.root}> | ||
<MenuButtonTooltip | ||
label={tooltipLabel} | ||
placement={tooltipPlacement} | ||
shortcutKeys={tooltipShortcutKeys} | ||
> | ||
<ToggleButton | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,14 +23,18 @@ export type MenuControlsContainerProps = { | |
* interval, if `debounced` is true. | ||
*/ | ||
DebounceProps?: Except<DebounceRenderProps, "children">; | ||
direction?: "horizontal" | "vertical"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a doc comment |
||
}; | ||
|
||
const useStyles = makeStyles({ | ||
const useStyles = makeStyles<{ | ||
direction: MenuControlsContainerProps["direction"]; | ||
}>({ | ||
name: { MenuControlsContainer: MenuControlsContainer }, | ||
})((theme) => { | ||
})((theme, { direction }) => { | ||
return { | ||
root: { | ||
display: "flex", | ||
flexDirection: direction === "vertical" ? "column" : "row", | ||
rowGap: theme.spacing(0.3), | ||
columnGap: theme.spacing(0.3), | ||
alignItems: "center", | ||
|
@@ -45,8 +49,9 @@ export default function MenuControlsContainer({ | |
className, | ||
debounced, | ||
DebounceProps, | ||
direction = "horizontal", | ||
}: MenuControlsContainerProps) { | ||
const { classes, cx } = useStyles(); | ||
const { classes, cx } = useStyles({ direction }); | ||
const content = <div className={cx(classes.root, className)}>{children}</div>; | ||
return debounced ? ( | ||
<DebounceRender {...DebounceProps}>{content}</DebounceRender> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,193 @@ | ||
import BorderStyle from "@mui/icons-material/BorderStyle"; | ||
import { ClickAwayListener, Fade, Paper, Popper } from "@mui/material"; | ||
import * as React from "react"; | ||
import { makeStyles } from "tss-react/mui"; | ||
import { useRichTextEditorContext } from "../context"; | ||
import { | ||
BorderStyleDashed, | ||
BorderStyleDotted, | ||
BorderStyleDouble, | ||
BorderStyleGroove, | ||
BorderStyleInset, | ||
BorderStyleNone, | ||
BorderStyleOutset, | ||
BorderStyleRidge, | ||
BorderStyleSolid, | ||
} from "../icons"; | ||
import { Z_INDEXES } from "../styles"; | ||
import MenuButton from "./MenuButton"; | ||
import type { MenuButtonTooltipProps } from "./MenuButtonTooltip"; | ||
import MenuControlsContainer from "./MenuControlsContainer"; | ||
|
||
export type BorderStyleSelectOption = { | ||
value: string; | ||
IconComponent: React.ElementType<{ | ||
className: string; | ||
}>; | ||
label?: string; | ||
shortcutKeys?: MenuButtonTooltipProps["shortcutKeys"]; | ||
}; | ||
|
||
export type MenuSelectTableBorderStyleProps = { | ||
options?: BorderStyleSelectOption[]; | ||
label?: string; | ||
className?: string; | ||
}; | ||
|
||
const useStyles = makeStyles({ name: { MenuSelectTableBorderStyle } })( | ||
(theme) => ({ | ||
root: { | ||
zIndex: Z_INDEXES.BUBBLE_MENU, | ||
}, | ||
|
||
paper: { | ||
backgroundColor: theme.palette.background.default, | ||
}, | ||
Comment on lines
+43
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This leads to a lint warning currently, since it's not used: https://github.com/sjdemartini/mui-tiptap/actions/runs/7368820861/job/20510696875?pr=185#step:10:12 (I will probably update eslint so that it returns a failing status code even for warnings, since ideally CI would flag this more noisily and prevent warnings from being merged.) edit: now enforced as of #192 |
||
}) | ||
); | ||
|
||
const DEFAULT_BORDER_STYLE_OPTIONS: BorderStyleSelectOption[] = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it correct that many of these options don't display differently than "solid" with the default mui-tiptap styles that have borderWidth set to 1px, and they'd require >1px borders to be visually distinct? |
||
{ | ||
value: "solid", | ||
label: "Solid", | ||
IconComponent: BorderStyleSolid, | ||
}, | ||
{ | ||
value: "dashed", | ||
label: "Dashed", | ||
IconComponent: BorderStyleDashed, | ||
}, | ||
{ | ||
value: "dotted", | ||
label: "Dotted", | ||
IconComponent: BorderStyleDotted, | ||
}, | ||
{ | ||
value: "double", | ||
label: "Double", | ||
IconComponent: BorderStyleDouble, | ||
}, | ||
{ | ||
value: "groove", | ||
label: "Groove", | ||
IconComponent: BorderStyleGroove, | ||
}, | ||
{ | ||
value: "ridge", | ||
label: "Ridge", | ||
IconComponent: BorderStyleRidge, | ||
}, | ||
{ | ||
value: "inset", | ||
label: "Inset", | ||
IconComponent: BorderStyleInset, | ||
}, | ||
{ | ||
value: "outset", | ||
label: "Outset", | ||
IconComponent: BorderStyleOutset, | ||
}, | ||
{ | ||
value: "none", | ||
label: "None", | ||
IconComponent: BorderStyleNone, | ||
}, | ||
]; | ||
|
||
export default function MenuSelectTableBorderStyle({ | ||
options = DEFAULT_BORDER_STYLE_OPTIONS, | ||
label, | ||
className, | ||
}: MenuSelectTableBorderStyleProps) { | ||
const editor = useRichTextEditorContext(); | ||
const [anchorEl, setAnchorEl] = React.useState<null | HTMLElement>(null); | ||
const open = Boolean(anchorEl); | ||
const { classes, cx } = useStyles(); | ||
const handleClick = (event: React.MouseEvent<HTMLElement>) => { | ||
anchorEl ? handleClose() : setAnchorEl(event.currentTarget); | ||
}; | ||
const handleClose = () => { | ||
setAnchorEl(null); | ||
}; | ||
|
||
if (!editor?.isEditable) { | ||
return null; | ||
} | ||
|
||
const IconComponent = | ||
options.find( | ||
(option) => | ||
editor.getAttributes("tableHeader").borderStyle === option.value || | ||
editor.getAttributes("tableCell").borderStyle === option.value | ||
)?.IconComponent ?? BorderStyle; | ||
|
||
return ( | ||
<> | ||
<MenuButton | ||
tooltipLabel={label ?? "Border style"} | ||
IconComponent={IconComponent} | ||
onClick={handleClick} | ||
selected={open} | ||
aria-haspopup="true" | ||
aria-expanded={open ? "true" : undefined} | ||
aria-controls={open ? "border-style-menu" : undefined} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for being mindful of accessibility. I don't see the |
||
/> | ||
<Popper | ||
transition | ||
open={open} | ||
anchorEl={anchorEl} | ||
placement="bottom-start" | ||
className={cx(classes.root, className)} | ||
> | ||
{({ TransitionProps }) => ( | ||
<Fade {...TransitionProps} timeout={100}> | ||
<div> | ||
<ClickAwayListener | ||
mouseEvent="onMouseDown" | ||
touchEvent="onTouchStart" | ||
onClickAway={handleClose} | ||
> | ||
<Paper elevation={5}> | ||
<MenuControlsContainer | ||
className={className} | ||
direction="vertical" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clever to reuse this component for this purpose, with this new prop! |
||
> | ||
{options.map((option) => ( | ||
<MenuButton | ||
key={option.value} | ||
tooltipLabel={option.label ?? option.value} | ||
tooltipPlacement="right" | ||
IconComponent={option.IconComponent} | ||
onClick={() => | ||
editor | ||
.chain() | ||
.focus() | ||
.setCellAttribute("borderStyle", option.value) | ||
.updateAttributes("tableHeader", { | ||
borderStyle: option.value, | ||
}) | ||
Comment on lines
+165
to
+168
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I take it |
||
.run() | ||
} | ||
disabled={ | ||
!editor | ||
.can() | ||
.setCellAttribute("borderStyle", option.value) | ||
} | ||
selected={ | ||
editor.getAttributes("tableHeader").borderStyle === | ||
option.value || | ||
editor.getAttributes("tableCell").borderStyle === | ||
option.value | ||
} | ||
/> | ||
))} | ||
</MenuControlsContainer> | ||
</Paper> | ||
</ClickAwayListener> | ||
</div> | ||
</Fade> | ||
)} | ||
</Popper> | ||
</> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason this is pinned with 2.0.4 minimum instead of 2.0.0-beta.210 like the other dependencies? Would this not work if the end user only has 2.0.0 installed for instance?