Skip to content

feat: improve edra editor and toolbar functionality#31

Merged
Tsuzat merged 23 commits intoTsuzat:mainfrom
SoRobby:main
Mar 25, 2025
Merged

feat: improve edra editor and toolbar functionality#31
Tsuzat merged 23 commits intoTsuzat:mainfrom
SoRobby:main

Conversation

@SoRobby
Copy link

@SoRobby SoRobby commented Mar 22, 2025

  • Fix editor focus to correctly handle text selection, ensuring the bubble menu displays on double-click and drag.
  • Set cursor to the clicked text position, defaulting to the end of the document if needed.
  • Update toolbar logic: allow optional children, support ordered commands, and simplify the internal implementation.
  • Add allowedBubbleMenuCommands parameter to customize bubble menu behavior.
  • Introduce new editor props (showSlashCommands, allowedCommands) for additional configuration.
  • Improve parameter naming to align with tiptap conventions.
  • Fix QuickColor behavior to close upon color selection.
  • Update documentation and examples to reflect these changes.

SoRobby added 15 commits March 21, 2025 23:09
- For both shadcn and headless, user can define a list of allowed menubar commands to be visible.
- Fixed typo of headless component.
- Added feature to allow enabling/disabling of the slash command menu by adding property to the Edra component.
- Minor improvements to parameter naming convention to match tiptap docs.
- Toolbar commands are displayed in the order they appear in the list `allowedCommands` list.
- This allows special commands to components to be passed to the toolbar.
…ng to end of document

- Set editor focus and move cursor to click position, defaulting to document end if no text is found.
Copy link
Owner

@Tsuzat Tsuzat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QuickColor, by default, should not close on select. The reason is that, in this way, the user can try multiple options and change them instantaneously. Otherwise, the user might have to do multiple clicks for changing 2-3 colors.

What can be done, is introduce an optional prop, shouldCloseOnSelect which should be "false" by default.

Copy link
Owner

@Tsuzat Tsuzat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we try to move this getOrderedToolbarItems function in utils.ts if the implementation is same??

@Tsuzat
Copy link
Owner

Tsuzat commented Mar 23, 2025

I have a mixed feeling with allowed commands in both toolbar and bubble-menu. If the developer wants to customize them, they can easily change the code. adding allowed commands will introduce unnecessary complexity. While I understand that having them might be a good thing and this will give developers more flexibility, but in that case, one can simply change them as per their liking in their codebase. But this was an amazing work @SoRobby

Copy link
Owner

@Tsuzat Tsuzat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think focusEditor can be moved to utils.ts and then can be utilized in editor.svelte.

@SoRobby
Copy link
Author

SoRobby commented Mar 23, 2025

#31 (review): QuickColor, by default, should not close on select. The reason is that, in this way, the user can try multiple options and change them instantaneously. Otherwise, the user might have to do multiple clicks for changing 2-3 colors.

What can be done, is introduce an optional prop, shouldCloseOnSelect which should be "false" by default.

I see your perspective, and that makes sense; my goal was to mimic how Microsoft Word works to create an expected user experience. I removed this change from the codebase and the QuickColor will now stay open.


#31 (review): Can we try to move this getOrderedToolbarItems function in utils.ts if the implementation is same??

Agreed, moved to utils.ts.


#31 (review): I think focusEditor can be moved to utils.ts and then can be utilized in editor.svelte.

Agreed, moved to utils.ts.


#31 (comment): I have a mixed feeling with allowed commands in both toolbar and bubble-menu. If the developer wants to customize them, they can easily change the code. adding allowed commands will introduce unnecessary complexity. While I understand that having them might be a good thing and this will give developers more flexibility, but in that case, one can simply change them as per their liking in their codebase. But this was an amazing work @SoRobby

Yes, the code does become more difficult to modify. While adding these features (the allowedCommands), I was hesitant about this implementation approach. Instead, a better solution might be to allow <EdraToolbar /> to accept child commands, which would look something like this:

<EdraToolbar {editor}>
    <Bold {editor} />
    <Heading1 {editor />
    <Heading2 {editor />
    <FontSize {editor} />
    <QuickColor {editor} />
    ...
<EdraToolbar />

If no children exist all commands can be displayed by default

I personally prefer the children approach (as shown above), where each child command (e.g., <Bold {editor} /> would be it's own component). This approach offers easy extendibility of new commands, modifying existing commands, and allows devs to easily set the order of commands.

What is your thought on this?

Tsuzat
Tsuzat previously approved these changes Mar 23, 2025
Copy link
Owner

@Tsuzat Tsuzat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an amazing feature. Thanks for the contribution.

@Tsuzat
Copy link
Owner

Tsuzat commented Mar 23, 2025

I personally prefer the children approach (as shown above), where each child command (e.g., <Bold {editor} /> would be it's own component). This approach offers easy extendibility of new commands, modifying existing commands, and allows devs to easily set the order of commands.

This makes more sense. One can also use EdraToolBarIcon for consistency along with commands.

But again, if the developer wants to change the toolbar or bubble-menu, they can do it right in the code base itself. This is why Edra is installed as a component, so that with the base tiptap setup, one can change it how they like it.

@Tsuzat
Copy link
Owner

Tsuzat commented Mar 23, 2025

In my opinion, following changes are amazing and really add amazing features.

  • Fix editor focus to correctly handle text selection, ensuring the bubble menu displays on double-click and drag.
  • Set cursor to the clicked text position, defaulting to the end of the document if needed.
  • Improve parameter naming to align with tiptap conventions.
  • Update documentation and examples to reflect these changes.

Whereas this needs an attention,

  • Introduce new editor props (showSlashCommands, allowedCommands) for additional configuration.

In this changeset, we should be only introducing showSlashCommands and remove allowedCommands and its implementation.

What do you think @SoRobby ?

@Tsuzat Tsuzat dismissed their stale review March 23, 2025 14:48

Need more thorough review and discussion.

@Tsuzat
Copy link
Owner

Tsuzat commented Mar 23, 2025

Hi @SoRobby, following changesets is amazing and I'd like to merge them ASAP.

  • Set cursor to the clicked text position, defaulting to the end of the document if needed.
  • Improve parameter naming to align with tiptap conventions.
  • Update documentation and examples to reflect these changes.

For rest of the changes, we need to have some discussions.

Can you please create a separate pull request with these changesets? Also do change the cursor as the div will now have a role of button. You can make change in editor.css

.edra-editor, .tiptap {
    cursor: text;
}

@SoRobby
Copy link
Author

SoRobby commented Mar 24, 2025

I'll make another PR within the next day or so that address the feedback.

To continue the discussion of commands:

I personally prefer the children approach (as shown above), where each child command (e.g., <Bold {editor} /> would be it's own component). This approach offers easy extendibility of new commands, modifying existing commands, and allows devs to easily set the order of commands.

This makes more sense. One can also use EdraToolBarIcon for consistency along with commands.

But again, if the developer wants to change the toolbar or bubble-menu, they can do it right in the code base itself. This is why Edra is installed as a component, so that with the base tiptap setup, one can change it how they like it.

While editing the Edra component directly works well for certain cases, especially when you want a single, predefined editor setup.

I’ve found that in some projects, I need multiple editors with different sets of commands. In those scenarios, having a way to configure the toolbar externally (e.g., via children or props) is really helpful for maintaining flexibility and reducing duplication. This was the main motivation behind configuring the toolbar and other components to accept a set of commands.

With the children command approach:

<EdraToolbar {editor}>
    <Bold {editor} />
    <Heading1 {editor{ />
    <Heading2 {editor} />
    <FontSize {editor} />
    <QuickColor {editor} />
    ...
<EdraToolbar />

I believe with this approach devs can still easily customize the editor to fit their specific use cases, while being easily extendable and modifiable to fit multiple editors that have different command sets. This would also allow for some really neat functionality such as devs can potentially extend <Heading1 {editor} fontSize={"32px"} /> (or other command components); while I personally don't have a use case for this type of extendibility, I can see how others would find it useful.

So here is what I propose:
<EdraToolbar {editor}>:

  • Allow child commands.
  • If no child commands, by default list all commands.

<Edra {editor} />:

  • This will require more work and potentially other restructuring, but <Edra /> should also allow for child components, specifically <BubbleMenu /> which BubbleMenu /> could also accept command components. Here is an example of how it could look:
<Edra class="min-h-64 overflow-auto" bind:editor {content} {onUpdate}>
    <BubbleMenu {editor}>
        <Bold {editor} />
        <Heading1 {editor{ />
        <Heading2 {editor} />
        <FontSize {editor} />
        <QuickColor {editor} />
    </BubbleMenu>
<Edra />

What are your thoughts regarding this structure and these changes?

@Tsuzat
Copy link
Owner

Tsuzat commented Mar 24, 2025

That's make perfect sense to me. I think we need to do following things here.

  1. We can expose the edra commands for the developers to use them.
  2. We need to refactor editor.svelte and provide a way to let developer decide what they want to use with that. e.g. like SlashCommands, BubbleMenus and more...
  3. EdraToolbar should get an optinal children prop for developers to create their own Toolbar.

I'll try to implement these features.

@Tsuzat
Copy link
Owner

Tsuzat commented Mar 24, 2025

Hi @SoRobby, Just merged the following PR #32. Take a look

SoRobby added 2 commits March 24, 2025 13:12
…ntions

- Merged latest changes from main branch and refactored code
- Renamed `showBubbleMenus` to `showBubbleMenu` for consistency with tiptap terminology
- Renamed `showSlashCommand` to `showSlashCommands` to accurately reflect its behavior
- Corrected typo in `SearcnAndReplace`, changed to `SearchAndReplace`
- Removed `getOrderedToolbarItems` utility as a new child-component approach is now used
@SoRobby
Copy link
Author

SoRobby commented Mar 24, 2025

@Tsuzat The changes look great! I've merged them into my branch. Please hold off on merging this PR, I'm still working on docs and parameter naming.

I'll let you know when it's ready.

SoRobby added 4 commits March 24, 2025 17:03
- added `showLinkBubbleMenu` and `showTableBubbleMenu` properties to the Edra editor, both default to true
- updated documentation and examples extensively to reflect recent feature additions and configuration changes
- improved demo settings with more descriptive text for each option
- added <meta name="robots" content="index,follow"> for SEO improvements
- applied minor styling changes to the demo interface and toolbar components
@SoRobby
Copy link
Author

SoRobby commented Mar 25, 2025

@Tsuzat The PR is now ready to be merged. Thanks for your updates, they were great!

Recent pushes include: updated docs, corrected parameter and file misspellings, added meta robot tag for search engines, removed unused utils (that I had previously added).

@Tsuzat Tsuzat merged commit 79625ce into Tsuzat:main Mar 25, 2025
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants