-
Notifications
You must be signed in to change notification settings - Fork 505
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
feat: Add thumbnails support for files in grid view #2337
base: main
Are you sure you want to change the base?
Conversation
Introduces a new grid view mode for the files page, allowing users to switch between list and grid views. Includes: - New FilesGrid component - View mode toggle buttons - Shared props between list and grid views - Updated rendering logic to support both view modes
Adds multi-select functionality and improved keyboard navigation to the files page grid view: - Implement file selection with checkboxes - Add keyboard navigation between files - Create a new SelectedActions component for bulk operations - Update styling to support selection and focus states
- Adjust GridFile thumbnail opacity for better visual appearance
- Add select all checkbox for grid view - Move selected state management to parent component - Refactor file selection logic to support bulk and individual selection - Minor UI layout adjustments
- Implement text preview for text-based files in Grid File - Add support for reading file contents with doRead action - Enhance FileThumbnail to display text previews - Add localized "Select all entries" text across language files, this was done using google translate
Thank you for submitting this PR!
Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution. |
Hey @lidel, could you take a look at my PR when you get a chance? Would really appreciate your review! |
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.
- Move view mode buttons to Header component - Truncate file hash in grid view for better readability - Adjust Header layout and styling
- Add translation for view mode - Apply consistent height to view mode buttons in FilesPage - Adjust Header component margins and layout - Add responsive margin for breadcrumbs header
- Implement keyboard shortcut modal with comprehensive keyboard navigation - Add drop zone and drag-and-drop functionality for files in grid view - Enhance list file navigation and selection with improved keyboard controls - Add visual indicators for drag and drop interactions in grid view
hey @lidel, i've been able to resolve all feedbacks/asks |
moving 2px and changing border weight does not render well and makes ui jiggle. switching to static sizing + reusing dotted border convention from the old Files screen
we dont want to cause unnecessary work for people at https://app.transifex.com/ipfs/ipfs-webui/dashboard/
we do not use shadows in UI anywhere else, let's keep grid view UI simple and follow similar visual indicators as list view
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.
Looking good, thank you.
I've pushed small cosmetics to unify UI conventions in both views.
(the bold border which I requested caused UI to jiggle – I've reverted it and simplified UI to not move 2px around. This should work better on systems with unstable font rendering – removing surface for jiggling on some Linux boxes)
@SgtPooki mind taking a look with fresh set of eyes and merging if no concerns?
The thumbnails grid view can be enabled by clicking on this icon:
Oh great!, Thanks for the changes too @lidel |
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.
A few things here:
- new filenames should be kebab case
- new files should be typescript
- new components should be functional components
- translations should not include any changes to anything except
public/locales/en/
folder.. we will update other languages later.
@SgtPooki noted, i would proceed with these changes |
@george-hub331 hey, check out the branch now.. I converted to ts and fixed some errors.. the keyboard functionality might need a second eye from you, I think I broke something but i'm going to have to step onto something else for a bit |
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.
review of the latest code..
sometimes, hitting enter on a focused item doesn't execute when in grid view.
I would love to merge the shortcut helper into a generic component, and would have loved that to be a separate PR.. that increases the complexity of this one significantly.
i'm going two open two issues to track shortcut handling being combined into a single component that can be used by both list and grid view, but also an issue to track DOs and DONTs for PRs until we fully migrate to typescript
|
||
// Make sure .tsx and .ts extensions are properly prioritized | ||
// This ordering allows imports like './file.js' to resolve to './file.ts' or './file.tsx' | ||
config.resolve.extensions = ['.js', '.jsx', '.ts', '.tsx', '...'] | ||
|
||
// Enable resolving .js imports to .ts/.tsx files without specific aliases | ||
config.resolve.extensionAlias = { | ||
'.js': ['.js', '.ts', '.tsx'], | ||
'.jsx': ['.jsx', '.tsx'] | ||
} |
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.
this fixes an issue where TS and webpack and eslint fought against each other about import extensions..
ideally all imports should end in file extension ".js"
"src/files/file-preview/file-thumbnail.tsx", | ||
"src/files/type-from-ext/index.js", | ||
"src/files/files-grid/files-grid.tsx", | ||
"src/files/files-grid/grid-file.tsx", | ||
"src/files/file-icon/FileIcon.js", | ||
"src/files/pin-icon/PinIcon.js", | ||
"src/icons/GlyphDots.js", | ||
"src/components/checkbox/Checkbox.js", | ||
"src/icons/GlyphPin.js", | ||
"src/icons/GlyphPinCloud.js", | ||
"src/icons/GlyphSmallTick.js", | ||
"src/icons/GlyphDocText.js", | ||
"src/icons/GlyphDocPicture.js", | ||
"src/icons/GlyphDocMusic.js", | ||
"src/icons/GlyphDocMovie.js", | ||
"src/icons/GlyphDocGeneric.js", | ||
"src/icons/GlyphDocCalc.js", | ||
"src/icons/GlyphFolder.js", | ||
"src/icons/GlyphPinCloud.js", | ||
"src/icons/GlyphPin.js", | ||
"src/icons/StrokeCube.js", | ||
"src/files/type-from-ext/extToType.js" |
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.
ts needed these files in the project because they're being imported by other files.. we really need to finish up the TS migration in this project because it's getting messy
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.
attempt at an explanation is here: #2349
if (platform === 'mac' && keySymbols.mac[key]) return keySymbols.mac[key] | ||
if (platform !== 'mac' && keySymbols.other[key]) return keySymbols.other[key] |
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.
@lidel do you have key recommendations for linux here?
@george-hub331 would you mind throwing together an e2e test that validates grid functionalities? |
Closes #2301
This PR adds thumbnail support for files in the IPFS WebUI grid view, improving the visual browsing experience.
Changes:
The thumbnails are implemented with:
Testing: