Skip to content
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

fix(dashboard): in-app editor preview only call when tab is opened #7186

Merged
merged 4 commits into from
Dec 3, 2024

Conversation

LetItRock
Copy link
Contributor

@LetItRock LetItRock commented Dec 2, 2024

What changed? Why was the change needed?

In-App Editor Preview - the api request for the preview should only be called when the tab is opened or when the apply button is pressed.
In the attached video, there are 2x calls to the API because the React effect is called twice on the mount.

Screenshots

Screen.Recording.2024-12-02.at.23.42.05.mov
Screen.Recording.2024-12-02.at.23.34.56.mov
Screen.Recording.2024-12-02.at.23.02.44.mov
Screen.Recording.2024-12-02.at.23.00.38.mov

}, [editorValue]);

const previewResult = previewData?.result;
const preview = previewResult?.preview as InAppRenderOutput | undefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like this, but unfortunately, with the current implementation, we can't show the skeleton loader optimistically.
To have the types automatically picked we need to do check for the type:

if (previewData?.result?.type !== IN_APP) {
   // RENDER SKELETON
}

// RENDER IN_APP PREVIEW

This requires a lot of refactoring in this file.

Comment on lines +50 to +63
const dataRef = useDataRef({
workflowSlug,
stepSlug,
controlValues,
editorValue,
});

useEffect(() => {
previewStep({
workflowSlug: dataRef.current.workflowSlug,
stepSlug: dataRef.current.stepSlug,
data: { controlValues: dataRef.current.controlValues, previewPayload: JSON.parse(dataRef.current.editorValue) },
});
}, [dataRef, previewStep]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

on mount call the preview endpoint

Comment on lines +65 to +71
const previewStepCallback = useCallback(() => {
return previewStep({
workflowSlug,
stepSlug,
data: { controlValues, previewPayload: JSON.parse(editorValue) },
});
}, [workflowSlug, stepSlug, controlValues, editorValue, previewStep]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

used for the apply button click

Copy link

netlify bot commented Dec 2, 2024

Deploy Preview for dev-web-novu ready!

Name Link
🔨 Latest commit f17eec1
🔍 Latest deploy log https://app.netlify.com/sites/dev-web-novu/deploys/674e3c9f2983db0008e0906a
😎 Deploy Preview https://deploy-preview-7186.dashboard.novu-staging.co
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 2, 2024

Deploy Preview for novu-stg-vite-dashboard-poc ready!

Name Link
🔨 Latest commit 7df7c4d
🔍 Latest deploy log https://app.netlify.com/sites/novu-stg-vite-dashboard-poc/deploys/674dce8c741987000730434b
😎 Deploy Preview https://deploy-preview-7186.dashboard-v2.novu-staging.co
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 2, 2024

Deploy Preview for dashboard-v2-novu-staging ready!

Name Link
🔨 Latest commit f17eec1
🔍 Latest deploy log https://app.netlify.com/sites/dashboard-v2-novu-staging/deploys/674e3c9fa5b85100083b3f27
😎 Deploy Preview https://deploy-preview-7186.dashboard-v2.novu-staging.co
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

<div className="to-background absolute -bottom-3 h-16 w-full bg-gradient-to-b from-transparent to-80%" />
</div>

<InboxPreview isPreviewPending={isPreviewPending} previewData={previewData} />
<Accordion type="single" collapsible value={accordionValue} onValueChange={setAccordionValue}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't decide on removing the accordion here in case of API "graceful error" as it provides a weird user experience when the components is shown during the loading state and then disappears.

if (!isPreviewPending && !data?.result) {
return null;
const previewResult = previewData?.result;
if (isPreviewPending || previewData === undefined) {
Copy link
Contributor Author

@LetItRock LetItRock Dec 2, 2024

Choose a reason for hiding this comment

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

the preview endpoint should never respond with an error

@LetItRock LetItRock merged commit 3e19659 into next Dec 3, 2024
35 checks passed
@LetItRock LetItRock deleted the nv-4931-in-app-preview-improvements branch December 3, 2024 08:53
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