-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
chore: Plugin Action Editor Context #36187
Changes from all commits
393a060
b3fae4f
04d8475
f1cd097
fe2cda8
7df82ee
1b8259b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
import React, { | ||
type ReactNode, | ||
createContext, | ||
useContext, | ||
useMemo, | ||
} from "react"; | ||
import type { Action } from "entities/Action"; | ||
import type { Plugin } from "api/PluginApi"; | ||
import type { Datasource } from "entities/Datasource"; | ||
|
||
interface PluginActionContextType { | ||
action: Action; | ||
editorConfig: unknown[]; | ||
settingsConfig: unknown[]; | ||
plugin: Plugin; | ||
datasource?: Datasource; | ||
} | ||
|
||
// No need to export this context to use it. Use the hook defined below instead | ||
const PluginActionContext = createContext<PluginActionContextType | null>(null); | ||
|
||
interface ChildrenProps { | ||
children: ReactNode[]; | ||
} | ||
|
||
export const PluginActionContextProvider = ( | ||
props: ChildrenProps & PluginActionContextType, | ||
) => { | ||
const { action, children, datasource, editorConfig, plugin, settingsConfig } = | ||
props; | ||
|
||
// using useMemo to avoid unnecessary renders | ||
const contextValue = useMemo( | ||
() => ({ | ||
action, | ||
datasource, | ||
editorConfig, | ||
plugin, | ||
settingsConfig, | ||
}), | ||
[action, datasource, editorConfig, plugin, settingsConfig], | ||
); | ||
|
||
return ( | ||
<PluginActionContext.Provider value={contextValue}> | ||
{children} | ||
</PluginActionContext.Provider> | ||
); | ||
}; | ||
|
||
// By using this hook, you are guaranteed that the states are correctly | ||
// typed and set. | ||
// Without this, consumers of the context would need to keep doing a null check | ||
export const usePluginActionContext = () => { | ||
const context = useContext(PluginActionContext); | ||
if (!context) { | ||
throw new Error( | ||
"usePluginActionContext must be used within usePluginActionContextProvider", | ||
); | ||
} | ||
return context; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
import React from "react"; | ||
import { useLocation } from "react-router"; | ||
import { identifyEntityFromPath } from "../navigation/FocusEntity"; | ||
import { useSelector } from "react-redux"; | ||
import { | ||
getActionByBaseId, | ||
getDatasource, | ||
getEditorConfig, | ||
getPlugin, | ||
getPluginSettingConfigs, | ||
} from "ee/selectors/entitiesSelector"; | ||
import { PluginActionContextProvider } from "./PluginActionContext"; | ||
import { get } from "lodash"; | ||
import EntityNotFoundPane from "pages/Editor/EntityNotFoundPane"; | ||
import { getIsEditorInitialized } from "selectors/editorSelectors"; | ||
import Spinner from "components/editorComponents/Spinner"; | ||
import CenteredWrapper from "components/designSystems/appsmith/CenteredWrapper"; | ||
import { Text } from "@appsmith/ads"; | ||
|
||
interface ChildrenProps { | ||
children: React.ReactNode[]; | ||
} | ||
|
||
const PluginActionEditor = (props: ChildrenProps) => { | ||
const { pathname } = useLocation(); | ||
|
||
const isEditorInitialized = useSelector(getIsEditorInitialized); | ||
|
||
const entity = identifyEntityFromPath(pathname); | ||
const action = useSelector((state) => getActionByBaseId(state, entity.id)); | ||
|
||
const pluginId = get(action, "pluginId", ""); | ||
const plugin = useSelector((state) => getPlugin(state, pluginId)); | ||
|
||
const datasourceId = get(action, "datasource.id", ""); | ||
const datasource = useSelector((state) => getDatasource(state, datasourceId)); | ||
|
||
const settingsConfig = useSelector((state) => | ||
getPluginSettingConfigs(state, pluginId), | ||
); | ||
|
||
const editorConfig = useSelector((state) => getEditorConfig(state, pluginId)); | ||
|
||
if (!isEditorInitialized) { | ||
return ( | ||
<CenteredWrapper> | ||
<Spinner size={30} /> | ||
</CenteredWrapper> | ||
); | ||
} | ||
|
||
if (!action) { | ||
return <EntityNotFoundPane />; | ||
} | ||
|
||
if (!plugin) { | ||
return ( | ||
<CenteredWrapper> | ||
<Text color="var(--ads-v2-color-fg-error)" kind="heading-m"> | ||
Plugin not installed! | ||
</Text> | ||
</CenteredWrapper> | ||
); | ||
} | ||
|
||
if (!settingsConfig || !editorConfig) { | ||
return ( | ||
<CenteredWrapper> | ||
<Text color="var(--ads-v2-color-fg-error)" kind="heading-m"> | ||
Editor config not found! | ||
</Text> | ||
</CenteredWrapper> | ||
); | ||
} | ||
|
||
return ( | ||
<PluginActionContextProvider | ||
action={action} | ||
datasource={datasource} | ||
editorConfig={editorConfig} | ||
plugin={plugin} | ||
settingsConfig={settingsConfig} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We will remove that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I meant was the Module editors show only specific settings for each pluginType hence they are passed from top bottom to decouple and avoid if-else inside the editors. if the plan is to remove that then what's the plan to keep the editor config agnostic? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. Yup I have this considered. Will make this close to the PluginActionToolbar where this will be used |
||
> | ||
{props.children} | ||
</PluginActionContextProvider> | ||
); | ||
}; | ||
|
||
export default PluginActionEditor; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import React from "react"; | ||
|
||
const PluginActionForm = () => { | ||
return <div />; | ||
}; | ||
|
||
export default PluginActionForm; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default } from "./PluginActionForm"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import React from "react"; | ||
|
||
const PluginActionResponsePane = () => { | ||
return <div />; | ||
}; | ||
|
||
export default PluginActionResponsePane; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import React from "react"; | ||
|
||
const PluginActionToolbar = () => { | ||
return <div />; | ||
}; | ||
|
||
export default PluginActionToolbar; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
export { default as PluginActionEditor } from "./PluginActionEditor"; | ||
export { | ||
PluginActionContextProvider, | ||
usePluginActionContext, | ||
} from "./PluginActionContext"; | ||
export { default as PluginActionToolbar } from "./components/PluginActionToolbar"; | ||
export { default as PluginActionForm } from "./components/PluginActionForm"; | ||
export { default as PluginActionResponsePane } from "./components/PluginActionResponsePane"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import React from "react"; | ||
import { | ||
PluginActionEditor, | ||
PluginActionToolbar, | ||
PluginActionForm, | ||
PluginActionResponsePane, | ||
} from "PluginActionEditor"; | ||
|
||
const AppPluginActionEditor = () => { | ||
return ( | ||
<PluginActionEditor> | ||
<PluginActionToolbar /> | ||
<PluginActionForm /> | ||
<PluginActionResponsePane /> | ||
</PluginActionEditor> | ||
); | ||
}; | ||
|
||
export default AppPluginActionEditor; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import { default as CE_AppPluginActionEditor } from "ce/pages/Editor/AppPluginActionEditor/AppPluginActionEditor"; | ||
|
||
export default CE_AppPluginActionEditor; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default as AppPluginActionEditor } from "ee/pages/Editor/AppPluginActionEditor/AppPluginActionEditor"; |
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.
With this refactor, should we rely on the path to identify the action? Can we pass the action as a prop to the
PluginActionEditor
?Reason for asking this is the path has to conform to a particular standard for this editor to work (path should have :entityId or :queryId). If we make this agnostic then this editor can be rendered anywhere and has no dependency on the url
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.
Another suggestion: AFAIK each action now has a context object to define which parent it belongs to. Maybe we can use that instead of the URL. (I don't know if this functionality exists for packages however, need to confirm with someone from backend team)
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.
I will make this change in the next PR to update the PluginActionEditor