awcodes / scribble

MIT License
43 stars 7 forks source link

feat: Add support for automatically adding custom tools to profiles #38

Closed Log1x closed 4 months ago

Log1x commented 5 months ago

This adds support for adding tools to profiles as bubble, suggestion, or toolbar by default.

$this
    ->icon('heroicon-o-cube-transparent')
    ->label('Notice')
    ->type(ToolType::Block)
    ->renderedView(view: 'scribble-tools.notice')
    ->optionsModal(NoticeModal::class)
    ->suggestionTool()
    ->toolbarTool()
    ->bubbleTool();

Alternatively, I can change this to only affect DefaultProfile instead if needed (kinda leaning towards it, would also be less code).

This paired with #37 should hopefully improve DX when getting started making changes/generated tools take effect immediately.

From personal experience, I sort of feel like ->suggestionTool() should be default in the tool generation stub(s).

Another thing to consider would be maybe dropping the Tool suffix on the methods.

awcodes commented 5 months ago

How much trouble would it be to have it default to the DefaultProfile but allow passing an array of profiles for it to be added to?

Log1x commented 5 months ago

Shouldn't be an issue. Will push an update soon!

awcodes commented 5 months ago

Sorry. I pushed some changes that are going to conflict with this. My fault. Should have waited on you.

Log1x commented 5 months ago

Sorry. I pushed some changes that are going to conflict with this. My fault. Should have waited on you.

No problem. I'm almost done with the implementation and I'll fix the conflicts.

Log1x commented 5 months ago

everything is done and should be working. could alternatively use Arr::wrap() on the getters and allow the profiles to be a string on the property instead of my is_string check in the setters. Let me know if you want that changed at all. Might be better to do it this way so closures can be strings too.

otherwise, profiles can be passed as a class string or an array of classes.

awcodes commented 5 months ago

Awesome. I appreciate it.

awcodes commented 5 months ago

I do think Arr:wrap() would be cleaner.

Log1x commented 5 months ago

I do think Arr:wrap() would be cleaner.

agreed. will push the change momentarily.

Log1x commented 5 months ago

done

awcodes commented 4 months ago

Thank you for your work on this. It will be in the next release.

I didn't want you to have to do it again because I keep creating conflicts for you. LOL.

I have tagged this in the PR to make sure you still get credit.