Show placeholder icons if not signed in or not owned sandbox #2164
Conversation
|
Nice! If you go to https://pr2164.build.csb.dev/s/new and click on the "Deployment" icon, you get an "An unknown error occurred when connecting to ZEIT" error. |
|
Good catch! Fixed it by refactoring to hooks and making the check use the same condition. |
|
Nice one @CompuIves! 👊 Will interfere with #1969 (which is here already some time) tho, so maybe that one should be merged first? 🤔 |
|
Good point! Let's merge the other one first. Did you test all the deployment flows in the PR? |
MichaelDeBoey
left a comment
MichaelDeBoey
left a comment
There was a problem hiding this comment.
Did a small review on the non-Deployment part 🙂
| } | ||
|
|
||
| const items = [PROJECT, FILES]; | ||
| const isCustomTemplate = !!store.editor.currentSandbox.customTemplate; |
There was a problem hiding this comment.
| const isCustomTemplate = !!store.editor.currentSandbox.customTemplate; | |
| const isCustomTemplate = Boolean(store.editor.currentSandbox.customTemplate); |
|
|
||
| interface IconProps { | ||
| item: INavigationItem; | ||
| store: any; |
There was a problem hiding this comment.
store isn't a prop, since you get it from the useStore hook
| store: any; |
| item: INavigationItem; | ||
| store: any; | ||
| isDisabled?: boolean; | ||
| setWorkspaceHidden: (params: { hidden: boolean }) => void; |
There was a problem hiding this comment.
We can just get setWorkspaceHidden from the useSignals hook instead, since we're just passing it from there in the parent component (Navigation).
| setWorkspaceHidden: (params: { hidden: boolean }) => void; |
| store: any; | ||
| isDisabled?: boolean; | ||
| setWorkspaceHidden: (params: { hidden: boolean }) => void; | ||
| setWorkspaceItem: (params: { item: string }) => void; |
There was a problem hiding this comment.
We can just get setWorkspaceItem from the useSignals hook instead, since we're just passing it from there in the parent component (Navigation).
| setWorkspaceItem: (params: { item: string }) => void; |
| } | ||
|
|
||
| const IconComponent = observer( | ||
| ({ item, isDisabled, setWorkspaceHidden, setWorkspaceItem }: IconProps) => { |
There was a problem hiding this comment.
| ({ item, isDisabled, setWorkspaceHidden, setWorkspaceItem }: IconProps) => { | |
| ({ item, isDisabled }: IconProps) => { |
| ({ item, isDisabled, setWorkspaceHidden, setWorkspaceItem }: IconProps) => { | ||
| const { id, name } = item; | ||
| const store = useStore(); | ||
|
|
There was a problem hiding this comment.
| const { | |
| workspace: { setWorkspaceHidden, setWorkspaceItem }, | |
| } = useSignals(); | |
There was a problem hiding this comment.
Thank you! Much cleaner!
| }; | ||
|
|
||
| export default function getItems(store) { | ||
| export function getDisabledItems(store: any): INavigationItem[] { |
There was a problem hiding this comment.
Maybe create a custom hook instead of passing the store? 🤔
There was a problem hiding this comment.
This one is used in multiple places, to make it reusable outside of React I think the store given is a nice solution.
| return []; | ||
| } | ||
|
|
||
| export default function getItems(store: any): INavigationItem[] { |
There was a problem hiding this comment.
Maybe create a custom hook instead of passing the store? 🤔
| <IconComponent | ||
| key={item.id} | ||
| item={item} | ||
| store={store} |
There was a problem hiding this comment.
store, setWorkspaceHidden & setWorkspaceItem can be deleted here (see above).
People often don't know what you can do with CodeSandbox because they're not signed in or they don't own the sandbox. This change is an experiment that should increase the discoverability of these features by showing a "disabled" version of the button if necessary:
When clicking on it they see this: