-
Notifications
You must be signed in to change notification settings - Fork 25
fix: embed mode click behavior #1641
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
fb83f67 to
4ce8fa1
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 fixes click behavior issues in embed mode by consolidating shared logic between table and tiles views, addressing problems with file selection, navigation, and the file picker functionality.
Key Changes:
- Introduced
useResourceViewHelperscomposable to centralize click handling logic for both ResourceTable and ResourceTiles components - Fixed tile view to properly handle file picker mode by hiding the select-all checkbox and preventing unintended file downloads
- Removed deprecated
useToggleTilecomposable in favor of the new shared helper functions
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/web-pkg/src/composables/resources/useResourceViewHelpers.ts |
New composable consolidating click handling logic for file containers, file names, and checkboxes |
packages/web-pkg/src/composables/resources/index.ts |
Exports new useResourceViewHelpers composable |
packages/web-pkg/src/composables/selection/useToggleTile.ts |
Removed deprecated composable, replaced by useResourceViewHelpers |
packages/web-pkg/src/composables/selection/index.ts |
Removed export of deprecated useToggleTile |
packages/web-pkg/src/components/FilesList/ResourceTiles.vue |
Refactored to use new helper composable, added file picker checkbox hiding |
packages/web-pkg/src/components/FilesList/ResourceTile.vue |
Updated event emissions to use new naming convention |
packages/web-pkg/src/components/FilesList/ResourceTable.vue |
Refactored to use new helper composable, simplified click handlers |
packages/web-pkg/tests/unit/components/FilesList/ResourceTiles.spec.ts |
Updated test to reflect new event emission names |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/web-pkg/src/composables/resources/useResourceViewHelpers.ts
Outdated
Show resolved
Hide resolved
Fixes several issues with the click behavior in the embed mode. - hide select all checkbox in tiles view with file picker enabled - fix click behavior in tiles view with embed enabled - make tiles not selectable via keyboard with file picker enabled - prevent files from getting downloaded when selecting them in table view with file picker enabled Moves quite a bit of shared logic between the table and the tiles view into one helper composable.
4ce8fa1 to
6d952a2
Compare
| :src="resource.thumbnail" | ||
| :data-test-thumbnail-resource-name="resource.name" | ||
| @click="toggleTile([resource, $event])" | ||
| @click.stop="$emit('tileClicked', [resource, $event])" |
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 don't understand why tileClicked exists, it is never consumed anywhere 🙈
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.
It's consumed in the ResourceTiles component.
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.
Ah, didn't check for @tile-clicked... -.- sorry for the noise
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.
😍
fix: embed mode click behavior
Fixes several issues with the click behavior in the embed mode.
Moves quite a bit of shared logic between the table and the tiles view into one helper composable. Mainly the ones for clicking files (or their containers/checkboxes).
fixes #1628
part of #1518