Skip to content

feat: Support customising canvas actions 🎉 #3364

Merged
ad1992 merged 19 commits into
masterfrom
support-hiding-ui-elements-1
Apr 4, 2021
Merged

feat: Support customising canvas actions 🎉 #3364
ad1992 merged 19 commits into
masterfrom
support-hiding-ui-elements-1

Conversation

@h7y
Copy link
Copy Markdown
Member

@h7y h7y commented Mar 29, 2021

I've used the action names for canvasActions attributes to conditionally disable the respective action, except for export since it doesn't have an action. I'm not sure whether this is the right way to do it. Looking for feedback :)

I'll add tests & make changes to docs after initial feedback.

Example with export hidden

Screenshot 2021-03-29 at 8 39 35 PM

Addresses 1/3 of #3012

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 29, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/excalidraw/excalidraw/5bCyps1CiVWQF32Pjyb16W4rUXmf
✅ Preview: https://excalidraw-git-support-hiding-ui-elements-1-excalidraw.vercel.app

@h7y h7y requested review from ad1992 and dwelle March 29, 2021 16:20
Comment thread src/types.ts Outdated
Comment thread src/actions/manager.tsx Outdated
Comment thread src/appState.ts Outdated
Comment thread src/components/App.tsx Outdated
@ad1992 ad1992 added this to the @excalidraw/excalidraw@0.6.0 milestone Mar 31, 2021
Copy link
Copy Markdown
Member

@ad1992 ad1992 left a comment

Choose a reason for hiding this comment

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

Since these prop will be one time options (only applicable during mount) I am thinking if we should instead rename the prop to

UIOptions: {
   canvasActions: {}
   }
 }

and later shapes and selectedShapeActions can also be added to the same instead of having individual props

Comment thread src/components/LayerUI.tsx Outdated
Comment thread src/components/LayerUI.tsx Outdated
Comment thread src/types.ts Outdated
Comment thread src/constants.ts Outdated
@h7y h7y requested a review from ad1992 April 1, 2021 19:20
@h7y h7y changed the title feat: Support hiding save, save as, clear canvas & export feat: Support hiding save, save as, clear canvas, load & export Apr 1, 2021
@h7y
Copy link
Copy Markdown
Member Author

h7y commented Apr 2, 2021

Perfect! I'll add tests and make changes to docs.

Copy link
Copy Markdown
Member

@ad1992 ad1992 left a comment

Choose a reason for hiding this comment

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

Screenshot 2021-04-02 at 12 59 41 PM

The spacing looks weird when only loadScene is allowed and collab is supported

I am ok with not disabling drag and drop when loadScene is false.

color picker and theme also comes under canvas actions so lets add background, theme attributes under canvas actions for that.

Comment thread src/actions/manager.tsx
.filter(
(action) =>
(action.name in canvasActions
? canvasActions[action.name as keyof typeof canvasActions]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The consumer will still be able to pass the actions which we don't support unless using typescript. I am ok with it to start with, we can whitelist it if needed later to prevent it.

@ad1992 ad1992 changed the title feat: Support hiding save, save as, clear canvas, load & export feat: Support customising canvas actions 🎉 Apr 2, 2021
Comment thread src/tests/excalidrawPackage.test.tsx Outdated
Comment thread src/tests/excalidrawPackage.test.tsx
@ad1992
Copy link
Copy Markdown
Member

ad1992 commented Apr 2, 2021

color picker and theme also comes under canvas actions so lets add background, theme attributes under canvas actions for that.

I added a prop for background. I see that theme is already controlled by the theme prop, should we add this anyway or should we move that prop to canvasActions?

Yep the theme prop is for whether the host wants to control it, this is for the host wants this functionality at all (nothing about controlling it from host) so we should support it, else host will have to unnecessary pass theme prop as a hack for not showing the theme button. However the logic wise on the code side it will be same (don't allow theme to be updated when this prop is false which means don't show the theme button)

Comment thread src/tests/excalidrawPackage.test.tsx Outdated
Comment thread src/packages/excalidraw/README.md Outdated
Comment thread src/packages/excalidraw/CHANGELOG.md Outdated
@ad1992
Copy link
Copy Markdown
Member

ad1992 commented Apr 4, 2021

Screenshot 2021-04-02 at 12 59 41 PM

The spacing looks weird when only loadScene is allowed and collab is supported

This we will have to check how we can we fix it as the spacing looks little weird when just one option is shown along with collab. But it can go in a separate PR as an improvement.

@ad1992 ad1992 merged commit 2335766 into master Apr 4, 2021
@ad1992 ad1992 deleted the support-hiding-ui-elements-1 branch April 4, 2021 10:27
@ad1992
Copy link
Copy Markdown
Member

ad1992 commented Apr 4, 2021

Thanks a lot for working on this @h7y ❤️

@h7y
Copy link
Copy Markdown
Member Author

h7y commented Apr 4, 2021

Thank you so much for the review @ad1992! ❤️

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.

3 participants