-
Notifications
You must be signed in to change notification settings - Fork 25
feat: add drilldown menu for sub menus on mobile devices #1017
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
Conversation
b9ff1fd to
bb926b4
Compare
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.
Pull Request Overview
This PR adds drilldown menu functionality for sub-menus on mobile devices by implementing a nested bottom drawer system. The main purpose is to enhance mobile user experience by providing proper navigation between menu levels on touch devices.
- Implements nested bottom drawer support with parent-child relationships
- Updates drop menu components to pass dropRef for better mobile integration
- Adds proper accessibility attributes and navigation controls for nested menus
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packages/web-runtime/src/layouts/Application.vue |
Enables multiple portal targets for bottom drawer rendering |
packages/web-pkg/src/components/ContextActions/*.vue |
Updates context action components to support nested drop references |
packages/web-pkg/src/components/FilesList/*.vue |
Passes dropRef through slot props for mobile menu integration |
packages/design-system/src/components/OcDrop/OcDrop.vue |
Refactors to support nested elements with parent references |
packages/design-system/src/components/OcBottomDrawer/OcBottomDrawer.vue |
Implements nested drawer functionality with back navigation |
| Test snapshots and documentation | Updates reflecting the new nested drawer behavior and API changes |
Comments suppressed due to low confidence (1)
packages/design-system/src/components/OcDrop/OcDrop.vue:150
- [nitpick] The prop name 'isNestedElement' is inconsistent with the previous 'isNested' naming pattern. Consider keeping 'isNested' for consistency or updating all related documentation and comments to use 'isNestedElement' terminology consistently.
isNestedElement = false,
| const bottomDrawerRef = useTemplateRef<HTMLElement | null>('bottomDrawerRef') | ||
| const bottomDrawerCardBodyRef = useTemplateRef<HTMLElement | null>('bottomDrawerCardBodyRef') | ||
| const open = async () => { | ||
| show.value = true | ||
| emit('open') | ||
| const show = async () => { | ||
| if (isNestedElement) { | ||
| unref(nestedParentRef).getElement().classList.add('oc-hidden') |
Copilot
AI
Aug 5, 2025
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.
Direct DOM manipulation using classList should be avoided in Vue components. Consider using Vue's reactive CSS classes or computed properties to manage visibility states instead of directly manipulating the DOM.
| const bottomDrawerRef = useTemplateRef<HTMLElement | null>('bottomDrawerRef') | |
| const bottomDrawerCardBodyRef = useTemplateRef<HTMLElement | null>('bottomDrawerCardBodyRef') | |
| const open = async () => { | |
| show.value = true | |
| emit('open') | |
| const show = async () => { | |
| if (isNestedElement) { | |
| unref(nestedParentRef).getElement().classList.add('oc-hidden') | |
| const isParentHidden = ref(false) | |
| const bottomDrawerRef = useTemplateRef<HTMLElement | null>('bottomDrawerRef') | |
| const bottomDrawerCardBodyRef = useTemplateRef<HTMLElement | null>('bottomDrawerCardBodyRef') | |
| const show = async () => { | |
| if (isNestedElement) { | |
| isParentHidden.value = true |
| unref(bottomDrawerCardBody)?.removeEventListener('click', onBottomDrawerChildClicked) | ||
| const openParentDrawer = () => { | ||
| hide({ hideParent: false }) | ||
| unref(nestedParentRef).getElement().classList.remove('oc-hidden') |
Copilot
AI
Aug 5, 2025
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.
Direct DOM manipulation using classList should be avoided in Vue components. Consider using Vue's reactive CSS classes or computed properties to manage visibility states instead of directly manipulating the DOM.
| unref(nestedParentRef).getElement().classList.remove('oc-hidden') | |
| unref(nestedParentRef).show() |
kulmann
left a comment
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 have some "Gemotze" regarding the aria-labels, but that's not in scope for this PR. More for the bottom drawer in general. I'll open a separate issue about that.
Nice job! UI/UX feels good as well. 🥳 💪
Description
Related Issue
How Has This Been Tested?
Types of changes