Skip to content

Unmount UModal or USlideover when closed #3157

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

Closed
robinsimonklein opened this issue Jan 22, 2025 · 8 comments · Fixed by #3279
Closed

Unmount UModal or USlideover when closed #3157

robinsimonklein opened this issue Jan 22, 2025 · 8 comments · Fixed by #3279
Labels
question Further information is requested v3 #1289

Comments

@robinsimonklein
Copy link
Contributor

Description

Hi there,
First of all, thanks for the amazing work on the v3, it looks really promising !

I'm struggling with USlideover in a CRUD app. What I'm trying to build is a table listing posts (using UTable), from which you can open post details in a USlideover, or open a form to edit this post also in a USlideover. I feel like the programmatic usage of USlideover (with useSlideover and a dedicated component) would be the most appropriate. This would also allow me to open posts info or edit form from elsewhere in the app.

But I can't get it to work properly. In fact, since the component only mounts once when opening the Slideover for the first time, it seems difficult to change the content at the right time without using tricky workarounds.

So the question : is there already a way to force USlideover to mount/unmount on open/close ? Or should we implement this as a feature (i.e: a unmount prop) ?

Or, am I doing it wrong and there is a more convenient way to do this ?

Here you can find a small and basic reproduction with just the "show" details slideover : https://codesandbox.io/p/github/robinsimonklein/nuxt-ui-3-crud-slideover/main?import=true or https://github.com/robinsimonklein/nuxt-ui-3-crud-slideover

Thanks for your help!

@robinsimonklein robinsimonklein added question Further information is requested v3 #1289 labels Jan 22, 2025
@PHPfanatic
Copy link

PHPfanatic commented Jan 23, 2025

I had this issue also and found that using .reset() after closing and/or before opening my next slideover with different content worked for me.
This is an example of a function that I use to accomplish this.

const userSlideOver = useSlideover()

function openEditUser() {
  userSlideOver.open(UserEditSlideover, {
    'userData': activeUser,
    onSuccess() {
      userRefresh()
      userSlideOver.close()
      userSlideOver.reset()
    }
  })
}

I'm calling reset within my onSuccess callback, however there should not be a reason that you cannot call it prior to firing just before your open command.

Here is an additional example where I call reset outside of the callback.

const userSlideOver = useSlideover()

const handleUserSelection = (event: Event) => {
  const target = event.target as HTMLInputElement
  activeUser.value = userRecords.value.find(user => user.uniqueId === target.getAttribute('data-id')) || null
  userSlideOver.reset()
}

// now I can call userSlideOver.open and my new user that was selected information is visible.

Hope this helps.

@schillerenrico
Copy link

schillerenrico commented Jan 23, 2025

@robinsimonklein For me, I am using :key on the component <USlideover :key="random"> which forces to rerender it each time the key changes

Maybe this is related to it as well #3105

@genu
Copy link
Contributor

genu commented Feb 13, 2025

@robinsimonklein Are you able to test this issue with this PR: #3279

It should solve this issue.

Overlays automatically mount/unmount when they are opened and closed.

@vitalijalbu
Copy link

vitalijalbu commented Feb 14, 2025

ah ok, that's why i passed json objs of users and it showed up only the 1st one, guys can anyone give me a honest review about my slideover+datatable (csutom) createb by me (not vue expert):

https://github.com/vitalijalbu/nuxt-ui-starter/blob/main/src/views/pages/users/Index.vue

will appreciate it a lot (also if can give like a mark from 1-10 will be helpul):
thanks 🙏

@schillerenrico
Copy link

schillerenrico commented Feb 14, 2025

ah ok, that's why i passed json objs of users and it showed up only the 1st one, guys can anyone give me a honest review about my slideover+datatable (csutom) createb by me (not vue expert):

https://github.com/vitalijalbu/nuxt-ui-starter/blob/main/src/views/pages/users/Index.vue

will appreciate it a lot (also if can give like a mark from 1-10 will be helpul): thanks 🙏

here is some chat gpt for ya :)

You're doing a great job structuring your component, especially considering you're not a Vue expert. I'll break down your implementation into different aspects and give feedback along with an overall rating.

1. Code Structure & Organization (8/10)

✅ Strengths:

  • Clean and well-structured code with clear separation of concerns.
  • Proper use of script setup, making it more concise.
  • The logic for opening the drawer and modal is neatly handled.
  • The use of getUserColumns() to dynamically generate table columns is a nice abstraction.

🔧 Improvements:

  • The variable isDeleteModalOpen is declared but not used in the template. If it's not needed, remove it.
  • Consider renaming drawerOpen to something more descriptive, e.g., isDrawerOpen to align with isDeleteModalOpen.
  • The console.log statements should ideally be removed before production.

2. Readability & Maintainability (9/10)

✅ Strengths:

  • Variable and function names are clear and self-explanatory.
  • Code is properly spaced, making it easy to read.
  • Good use of TypeScript to ensure type safety.

🔧 Improvements:

  • Instead of inline functions (openDrawer, openDeleteModal), you could move them into a composable or Vue Store if they are reused elsewhere.
  • The t function is used correctly for translations, but if the component grows, it might be better to centralize keys instead of using inline translations.

3. Functionality & Best Practices (7.5/10)

✅ Strengths:

  • The datatable and drawer logic are well handled and reusable.
  • Use of v-model on drawerOpen is a clean way to handle state.
  • Proper use of default function parameters (openDrawer(user: User | null = null)).

🔧 Improvements:

  • The deletion logic is incomplete; there is no actual API call. Consider integrating a composable (e.g., useUsersStore() or useUsers()) for actions like delete/update.
  • The console.log('selected user', selectedUser.value) might be useful for debugging but should be removed or replaced with Vue DevTools debugging.
  • You could use defineProps() in DrawerUser instead of passing selectedUser as a separate prop with v-model.

4. UI & UX Considerations (8/10)

✅ Strengths:

  • The button to add a new user is positioned well inside the PageHeader.
  • The slideover approach is a good UX decision, as it allows users to edit without leaving the page.
  • Icons are used effectively (lucide:plus on the button).

🔧 Improvements:

  • Consider adding a loading state when opening the drawer for a smoother experience.
  • If the drawer is used for both create and edit, you might want to display a different title inside DrawerUser depending on whether selectedUser is null.

Overall Rating: 8/10

You're on the right track! Your implementation is clean, readable, and functional, and with a few refinements (handling deletion properly, improving naming, optimizing reusability), it can be even better.

@robinsimonklein
Copy link
Contributor Author

@genu Thanks for this PR!
I just tried to install it in my project by using npm i https://pkg.pr.new/@nuxt/ui@3279 as commented by the bot, but I get the following 500 error when loading any page of my app:

useOverlay is not defined

at _sfc_main.setup (./node_modules/@nuxt/ui/dist/runtime/components/OverlayProvider.js:70:23)
at callWithErrorHandling (./node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:200:19)
at setupStatefulComponent (./node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:7848:25)
at setupComponent (./node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:7809:36)
at renderComponentVNode (./node_modules/@vue/server-renderer/dist/server-renderer.cjs.js:645:15)
at renderVNode (./node_modules/@vue/server-renderer/dist/server-renderer.cjs.js:779:14)
at renderVNodeChildren (./node_modules/@vue/server-renderer/dist/server-renderer.cjs.js:795:5)
at renderVNode (./node_modules/@vue/server-renderer/dist/server-renderer.cjs.js:767:7)
at renderComponentSubTree (./node_modules/@vue/server-renderer/dist/server-renderer.cjs.js:730:7)

Am I doing something wrong ?

@genu
Copy link
Contributor

genu commented Feb 16, 2025

Hi @robinsimonklein I noticed similar issue when trying to test PRs with the pkg.pr.new links. I'm not sure why that hasn't been working.

Pinging @benjamincanac to maybe check that.

For now, you'd have to clone and checkout the PR branch to test, unfortunatellyu.

@benjamincanac
Copy link
Member

I'd recommend using the commit hash instead of the PR number when using pkg.pr.new: https://pkg.pr.new/@nuxt/ui@1b19c37 for #3279 latest commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested v3 #1289
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants