feat: Improve tiptap-integration of toolbars for better UX #43
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide by SourceryThis PR implements UX improvements by refactoring the toolbar code and moving it into its own module. The toolbars are now Prosemirror decorations within the editable div, improving focus and event handling. Toolbar visibility is managed with CSS, and the block toolbar now acts as a drag handle. Class diagram for the new toolbar systemclassDiagram
class CmsToolbarPlugin {
+addProseMirrorPlugins()
}
class Plugin {
+props
+state
}
class TiptapToolbar {
+action()
+enabled()
+active()
+type
}
class BlockToolbar {
+createBlockToolbar()
+updateBlockToolbar()
+handleToolbarClick()
}
class TopToolbar {
+createTopToolbar()
+updateToolbar()
+handleToolbarClick()
}
CmsToolbarPlugin --> Plugin
CmsToolbarPlugin --> BlockToolbar
CmsToolbarPlugin --> TopToolbar
BlockToolbar --> TiptapToolbar
TopToolbar --> TiptapToolbar
State diagram for toolbar visibility and interactionstateDiagram-v2
[*] --> Hidden: Initial State
Hidden --> Visible: Editor Focused
Visible --> Hidden: Editor Blurred
state Visible {
[*] --> Closed
Closed --> Open: Click Toolbar
Open --> Closed: Click Outside
Open --> DragMode: Start Drag
DragMode --> Closed: End Drag
}
note right of Hidden: Toolbar is invisible
note right of Visible: Toolbar shows with editor focus
note right of DragMode: Block toolbar acts as drag handle
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #43 +/- ##
=======================================
Coverage 81.16% 81.16%
=======================================
Files 17 17
Lines 929 929
Branches 104 104
=======================================
Hits 754 754
Misses 132 132
Partials 43 43 ☔ View full report in Codecov by Sentry. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @fsbraun - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Fix typo in mousedowxn event handler name (link)
Overall Comments:
- Consider moving the SVG icon definitions to separate asset files to improve code readability and maintainability
Here's what I looked at during the review
- 🔴 General issues: 1 blocking issue
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
…t into feat/pm-widgets # Conflicts: # private/js/tiptap_plugins/cms.toolbar.js
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @fsbraun - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -0,0 +1,563 @@ | |||
| /* eslint-env es11 */ | |||
There was a problem hiding this comment.
issue (complexity): Consider refactoring the toolbar creation and event handling to be more modular and reduce duplication.
The toolbar implementation has unnecessary complexity that can be reduced in two key areas:
- Break down the monolithic
_populateToolbarfunction into focused component builders:
function _createToolbarButton(editor, item, filter) {
// Existing button creation logic
}
function _createDropdown(editor, item, filter) {
const dropdown = typeof item.items === 'function'
? item.items(editor, items => _populateToolbar(editor, items, filter))
: _populateToolbar(editor, item.items, filter);
return `<span title='${item.title}' class="dropdown" role="button">
${item.icon}<div class="dropdown-content ${item.class || ''}">${dropdown}</div>
</span>`;
}
function _populateToolbar(editor, array, filter) {
return array.map(item => {
if (Array.isArray(item)) return _handleGroup(editor, item, filter);
if (item.constructor === Object) return _createDropdown(editor, item, filter);
if (item === '|') return editor.options.separator_markup;
return _createToolbarButton(editor, item, filter);
}).join('');
}- Consolidate duplicated event handling between block and top toolbar:
const sharedToolbarHandlers = {
handleClick(view, event, editor) {
if (this.containsTarget(event.target)) {
_handleToolbarClick(event, editor);
return true;
}
_closeAllDropdowns(event, editor);
return false;
},
containsTarget(target) {
return this.toolbar?.contains(target);
}
};
function createToolbarPlugin(editor, type) {
return new Plugin({
props: {
handleDOMEvents: {
click(view, event) {
return sharedToolbarHandlers.handleClick(view, event, editor);
}
}
}
// ... rest of plugin config
});
}These changes maintain all functionality while making the code more maintainable and easier to understand.
|
@sourcery-ai review |
fsbraun
changed the title
There was a problem hiding this comment.
Hey @fsbraun - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
UX improvements
This PR introduces a new toolbar system for improved user experience.
divas Prosemirror decorations. This avoids problems with timeouts when the editabledivblurs.dialogtag to adivtag to leverage the different focus concepts.Summary by Sourcery
New Features: