-
Notifications
You must be signed in to change notification settings - Fork 25
feat: improve tile sort menu drop and drawer design #1004
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
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 improves the design and functionality of the tile sort menu dropdown and drawer, following up on previous work in PR #997. The changes focus on enhancing the user experience through better visual design and improved interaction patterns.
- Updates the sort filter chip to use a proper list structure and improved styling
- Modifies the filter label and active state handling for better user feedback
- Refactors the test to use a more robust element selection approach
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/web-pkg/src/components/FilesList/ResourceTiles.vue | Restructures sort dropdown with proper list markup, updates styling classes, and improves active state display |
| packages/web-pkg/tests/unit/components/FilesList/ResourceTiles.spec.ts | Updates test to use findAll() with array indexing instead of CSS nth-child selector |
| <li v-for="(option, index) in sortFields" :key="index"> | ||
| <oc-button | ||
| appearance="raw" | ||
| :class="{ 'oc-secondary-container': currentSortField === option }" |
Copilot
AI
Jul 24, 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.
[nitpick] The conditional class 'oc-secondary-container' should be applied consistently. Consider using a computed property or method to determine the button's class state for better maintainability.
| &-item:hover:not(&-item-active) { | ||
| background-color: var(--oc-role-surface-container) !important; | ||
| &-item { | ||
| justify-content: space-between !important; |
Copilot
AI
Jul 24, 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.
Using !important should be avoided as it makes styles harder to maintain and override. Consider restructuring the CSS specificity or using more specific selectors instead.
| justify-content: space-between !important; | |
| display: flex; | |
| justify-content: space-between; |
Follow-up for #997.