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

feat(dashboard): in-app editor unsaved changes modal plus handling esc and clicking on drawer gray overlay #6958

Merged
merged 5 commits into from
Nov 12, 2024

Conversation

LetItRock
Copy link
Contributor

What changed? Why was the change needed?

In-App Editor Drawer:

  • unsaved changes modal
  • handle the ESC button and clicks on the drawer gray overlay

Screenshots

Screenshot 2024-11-12 at 13 56 58

Screen.Recording.2024-11-12.at.13.56.15.mov

Copy link

linear bot commented Nov 12, 2024

@@ -45,6 +45,7 @@
"@radix-ui/react-switch": "^1.1.1",
"@radix-ui/react-tabs": "^1.1.1",
"@radix-ui/react-tooltip": "^1.1.3",
"@radix-ui/react-visually-hidden": "^1.1.0",
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 to hide some required components, more about it in the comments below

@@ -55,7 +55,7 @@ TabsTrigger.displayName = TabsPrimitive.Trigger.displayName;
const tabsContentVariants = cva('focus-visible:outline-none', {
variants: {
variant: {
default: 'ring-offset-background focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2',
default: '',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no need to show the outline on the tabs content component

{/* TODO: show loading indicator */}
{workflow && step && stepType && <StepEditor workflow={workflow} step={step} stepType={stepType} />}
</motion.div>
<SheetContentBase asChild onInteractOutside={handleCloseSidebar} onEscapeKeyDown={handleCloseSidebar}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The SheetContentBase is the component that allows to handle the clicks on the drawer overlay and ESC button clicks. But the thing is that it requires the use of the SheetTitle, SheetDescription under the hood, but the current implementation doesn't use it. So I had to hide these two visually.
We might decide to refactor that in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a TODO in the code.

</Tabs>
</form>
</Form>
<AlertDialog open={blocker.state === 'blocked'}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The unsaved changes modal

Copy link

netlify bot commented Nov 12, 2024

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

Name Link
🔨 Latest commit 2c22c50
🔍 Latest deploy log https://app.netlify.com/sites/novu-stg-vite-dashboard-poc/deploys/6733886786ccbf0008c4ae5a
😎 Deploy Preview https://deploy-preview-6958--novu-stg-vite-dashboard-poc.netlify.app
📱 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.

Base automatically changed from nv-4513-workflow-editor-error-handling to next November 12, 2024 16:43
Copy link

pkg-pr-new bot commented Nov 12, 2024

Open in Stackblitz

@novu/client

pnpm add https://pkg.pr.new/novuhq/novu/@novu/client@6958

@novu/headless

pnpm add https://pkg.pr.new/novuhq/novu/@novu/headless@6958

@novu/framework

pnpm add https://pkg.pr.new/novuhq/novu/@novu/framework@6958

@novu/js

pnpm add https://pkg.pr.new/novuhq/novu/@novu/js@6958

@novu/nest

pnpm add https://pkg.pr.new/novuhq/novu/@novu/nest@6958

@novu/nextjs

pnpm add https://pkg.pr.new/novuhq/novu/@novu/nextjs@6958

@novu/node

pnpm add https://pkg.pr.new/novuhq/novu/@novu/node@6958

@novu/notification-center

pnpm add https://pkg.pr.new/novuhq/novu/@novu/notification-center@6958

novu

pnpm add https://pkg.pr.new/novuhq/novu@6958

@novu/providers

pnpm add https://pkg.pr.new/novuhq/novu/@novu/providers@6958

@novu/react

pnpm add https://pkg.pr.new/novuhq/novu/@novu/react@6958

@novu/react-native

pnpm add https://pkg.pr.new/novuhq/novu/@novu/react-native@6958

@novu/shared

pnpm add https://pkg.pr.new/novuhq/novu/@novu/shared@6958

@novu/stateless

pnpm add https://pkg.pr.new/novuhq/novu/@novu/stateless@6958

commit: 2c22c50

@LetItRock LetItRock merged commit aabc1df into next Nov 12, 2024
40 checks passed
@LetItRock LetItRock deleted the nv-4609-in-app-editor-unsaved-changes branch November 12, 2024 17:33
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.

4 participants