-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat: node actions #18248
feat: node actions #18248
Conversation
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
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.
Not sure why these broke...
@@ -87,7 +87,7 @@ export const TextOnly = (): JSX.Element => { | |||
} | |||
|
|||
export const Sizes = (): JSX.Element => { | |||
const sizes: LemonButtonProps['size'][] = ['small', 'medium', 'large'] | |||
const sizes: LemonButtonProps['size'][] = ['tiny' | 'small', 'medium', 'large'] |
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.
const sizes: LemonButtonProps['size'][] = ['tiny' | 'small', 'medium', 'large'] | |
const sizes: LemonButtonProps['size'][] = ['tiny', 'small', 'medium', 'large'] |
@@ -102,7 +102,7 @@ export const Sizes = (): JSX.Element => { | |||
} | |||
|
|||
export const SizesIconOnly = (): JSX.Element => { | |||
const sizes: LemonButtonProps['size'][] = ['small', 'medium', 'large'] | |||
const sizes: LemonButtonProps['size'][] = ['tiny' | 'small', 'medium', 'large'] |
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.
const sizes: LemonButtonProps['size'][] = ['tiny' | 'small', 'medium', 'large'] | |
const sizes: LemonButtonProps['size'][] = ['tiny', 'small', 'medium', 'large'] |
@@ -63,7 +63,7 @@ export interface LemonButtonPropsBase | |||
/** Like plain `disabled`, except we enforce a reason to be shown in the tooltip. */ | |||
disabledReason?: string | null | false | |||
noPadding?: boolean | |||
size?: 'small' | 'medium' | 'large' | |||
size?: 'tiny' | 'small' | 'medium' | 'large' |
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.
I wonder if xsmall
is better? Fits better with the tailwind-esque xs, sm, md, lg etc.
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.
Yep 👍 Ideally we'd move everything to the xs, sm, etc... legend
{actions.map((x, i) => ( | ||
<LemonButton | ||
key={i} | ||
size="tiny" |
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.
I think the below should just be
/> | ||
) | ||
}) | ||
export const SlashCommandsPopover = forwardRef<SlashCommandsRef, SlashCommandsPopoverProps>( |
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.
Something in here isn't working. With one node in the book, if I click add and pick something it replaces the node....
📸 UI snapshots have been updated3 snapshot changes in total. 0 added, 3 modified, 0 deleted:
Triggered by this commit. |
11f3e0b
to
14f593d
Compare
📸 UI snapshots have been updated3 snapshot changes in total. 0 added, 3 modified, 0 deleted:
Triggered by this commit. |
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.
Lookin good. I pushed a small icon change but otherwise lets goooo
Problem
The gap cursor logic didn't feel great the first time around
Still todo: Nice empty states to further encourage insertion of nodes in empty docs
Changes
SlashCommandsPopover
component more generic to support new insertion optionNodeGapInsertionExtension
LemonButtonWithDropdown
and add a linting rule on top of the deprecationHow did you test this code?
Made sure everything still worked manually