Skip to content

Conversation

@JammingBen
Copy link
Contributor

@JammingBen JammingBen commented Oct 28, 2025

Fixes several issues with the bottom drawer (especially with nested ones) and improves the general animation handling when opening/closing nested menus.

This was achieved by introducing a store that keeps track of all the registered bottom drawers. This should be a much cleaner approach than the previous one.

fixes #1444
fixes #1445
fixes #1451

@JammingBen JammingBen self-assigned this Oct 28, 2025
@JammingBen JammingBen changed the title fix: bottom drawer issues fix: bottom drawer nesting issues Oct 28, 2025
@JammingBen JammingBen force-pushed the fix/bottom-drawer-issues branch 3 times, most recently from d1395f6 to 45d9107 Compare October 28, 2025 14:05
@JammingBen JammingBen marked this pull request as ready for review October 28, 2025 14:19
Copilot AI review requested due to automatic review settings October 28, 2025 14:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the bottom drawer implementation to fix nesting issues by introducing a centralized store for managing drawer state. The previous approach of passing drawer references through props has been replaced with a Pinia store that tracks all active drawers, enabling proper animation handling and simplifying nested menu logic.

Key Changes:

  • Introduced a new useBottomDrawer Pinia store to centrally manage drawer lifecycle and state
  • Removed NestedDrop type and associated prop-drilling of drawer references throughout the component hierarchy
  • Simplified drawer nesting by leveraging the store's drawer stack instead of parent-child references

Reviewed Changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/design-system/src/composables/stores/bottomDrawer.ts New Pinia store managing drawer registration, current drawer tracking, and lifecycle methods
packages/design-system/src/components/OcBottomDrawer/OcBottomDrawer.vue Refactored to use the drawer store instead of manual parent/child reference management
packages/design-system/src/components/OcDrop/OcDrop.vue Removed nested drawer prop passing and simplified exposed API
packages/design-system/src/helpers/types.ts Removed deprecated NestedDrop type definition
packages/web-pkg/src/components/ContextActions/*.vue Removed dropRef props that are no longer needed
packages/web-app-files/src/views/spaces/*.vue Updated context menu slot bindings to remove dropRef
packages/design-system/package.json Added Pinia dependency
packages/design-system/src/components/OcBottomDrawer/OcBottomDrawer.spec.ts Updated tests to mock the new drawer store
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JammingBen JammingBen force-pushed the fix/bottom-drawer-issues branch from 45d9107 to e218ab9 Compare October 28, 2025 14:22
@AlexAndBear
Copy link
Contributor

Noticed a regression:

  1. Head to a space
  2. Click 3 dot menu
  3. Click Edit Image
  4. Click Set Icon
  5. Emoji Picker opens but moved to the background as the BottomDrawer doesn't close correctly

Fixes several issues with the bottom drawer (especially with nested ones) and improves the general animation handling when opening/closing nested menus.

This was achieved by introducing a store that keeps track of all the registered bottom drawers.
@JammingBen JammingBen force-pushed the fix/bottom-drawer-issues branch from e218ab9 to aba40f0 Compare November 3, 2025 08:56
@JammingBen
Copy link
Contributor Author

Noticed a regression:

  1. Head to a space
  2. Click 3 dot menu
  3. Click Edit Image
  4. Click Set Icon
  5. Emoji Picker opens but moved to the background as the BottomDrawer doesn't close correctly

Good catch, thx! Should be fixed 👍

Copy link
Contributor

@AlexAndBear AlexAndBear left a comment

Choose a reason for hiding this comment

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

Works like a charm 👍

@JammingBen JammingBen merged commit ad92776 into main Nov 3, 2025
28 checks passed
@JammingBen JammingBen deleted the fix/bottom-drawer-issues branch November 3, 2025 09:29
openclouders pushed a commit that referenced this pull request Nov 3, 2025
@openclouders openclouders mentioned this pull request Nov 3, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

3 participants