feat: support disabling image tool #6320
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
|
I had a quick look at the Preview. Works as expected except that the icon is still visible in the toolbar with no obvious indicator that it is disabled. I would take it as is but just thought I should flag that as I think it would be better. |
|
awesome @dwelle ... really hope thid can be merged, but as p2gdeveloper the preview seems to have the image button visible .. but it seems you can't add one via this... but if i drag &drop an image, it seems to accept it |
dwelle
force-pushed
the
dwelle/support-disabling-image-tool
branch
from
965411c
to
ba951b9
Compare
|
@eMerzh this PR doesn't disable the image tool (since we don't want to disable by default), so it will work in the preview. But I've pushed debug commit we'll remove before merge so we can test: https://excalidraw-ev2ennbl1-excalidraw.vercel.app/ As for when this PR can be merged, there are some things left to be figured out (namely importing files / links that already contain images, and what should we do in those cases). I've contacted the creator of the original PR to discuss our next steps: #5133 (comment) |
|
What are thoughts on this feature? Would love to see this capability |
|
i don't want to do much noise, but is there something new on this front? |
|
sorry, will take a look |
Ahah no need to apologize 😅...but I will be happy if this get merged of course 😊 |
dwelle
force-pushed
the
dwelle/support-disabling-image-tool
branch
from
d1eab5e
to
564c662
Compare
dwelle
force-pushed
the
dwelle/support-disabling-image-tool
branch
from
564c662
to
999bbfb
Compare
size-limit report 📦
|
src/packages/excalidraw/CHANGELOG.md
Outdated
| @@ -19,6 +19,8 @@ Please add the latest change on the top under the correct section. | |||
|
|
|||
| #### BREAKING CHANGE | |||
|
|
|||
| - Added support for disabling `image` tool (also disabling image insertion in general, though keeps support for importing from `.excalidraw` files) [#6320](https://github.com/excalidraw/excalidraw/pull/6320). | |||
There was a problem hiding this comment.
Breaking change isn't a separate type but a subtype and this is interfering with continuity of the feature excalidraw API breaking change.
This should be added under feature and then breaking change as a subtype under it similar to excalidrawAPI
There was a problem hiding this comment.
moved to feature as filing under breaking change was a mistake. There's no breaking change with this API.
That said, we should be structuring our breaking changes differently, and if nothing else, list them first and that's the most important thing for host apps when reading a changelog.
There was a problem hiding this comment.
That said, we should be structuring our breaking changes differently, and if nothing else, list them first and that's the most important thing for host apps when reading a changelog.
When breaking change is as result of some feature / fix then it should be a sub heading so its clear why it was introduced and how to deal with it instead of separating features/fixes and breaking change separately. Even when its a subheading its going to catch the attention as well and also explain the context better due to main heading.
EDIT: Thinking again since we have been pushed breaking changes to make the API better in that case its not good to split it across multiple headings so I will club into one heading "Breaking Changes" instead so all breaking changes are at one place
Co-authored-by: Aakansha Doshi <aakansha1216@gmail.com>
Co-authored-by: Aakansha Doshi <aakansha1216@gmail.com>
fix #4831
Disables
imagetool/image insertion whenprops.UIOptions.tools.image = false.Non-goals:
doesn't affect tool keyboard shortcuts -> if image tool is disabled, there'll be a hole in the numeric shortcuts:
does not disable importing
.excalidrawfiles that contain images. We may add support for disabling later after we decide on API and what should happen.