-
Notifications
You must be signed in to change notification settings - Fork 109
fix(FloatingButtons): group smartpicker button and drag handle together #7818
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
Conversation
- The button group follows the cursor now - The button group is top-aligned to hovered paragraph node - The drag handle button has cursor type `grab` - Not displayed on mobile and full width view Also removes block margin from link previews. We already have margin between paragraphs and it makes the floating buttons look off otherwise. Fixes: #7272 Fixes: #7604 Signed-off-by: Jonas <[email protected]>
8236b6d to
2e44814
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7818 +/- ##
==========================================
+ Coverage 52.96% 59.72% +6.75%
==========================================
Files 501 499 -2
Lines 43372 38327 -5045
Branches 1092 1061 -31
==========================================
- Hits 22973 22890 -83
+ Misses 20293 15332 -4961
+ Partials 106 105 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| class="floating-buttons" | ||
| :class="{ heading: isHeadingNode }" | ||
| :on-node-change="onNodeChange"> | ||
| <NcButton |
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.
Seems a bit weird to have the plus button wrapped in the drag handle itself. I guess that is a bit of a shortcut to have a more straight forward implementation. Only "issue" i could see is that you can use the plus button as a drag handle as well. Not a blocker for me though
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.
Hehe, first I thought the same. Then I realized that it's exactly what the "notion style" Tiptap example does it as well 🤪
I didn't see a good reason to not do it and actually it results in cleaner code, as I can reuse the node and pos returned by the drag handler callback for the plus button action.
|
/backport to stable32 |
|
/backport to stable32 |
grabAlso removes block margin from link previews. We already have margin between paragraphs and it makes the floating buttons look off otherwise.
Fixes: #7272
Fixes: #7604
🖼️ Screenshots
🖼️ Screencast
🏁 Checklist
npm run lint/npm run stylelint/composer run cs:check)