feat: Asset picker for XBlocks #3130
Conversation
|
Thanks for the pull request, @xitij2000! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
|
@xitij2000 Could you rebase? It looks like this has conflicts. |
Kelketek
left a comment
There was a problem hiding this comment.
@xitij2000 Found some code style issues that can probably be autofixed by the linter.
Had one suggestion for a light refactor.
I'm still getting the detatched footer for this. I'm really curious if it will happen for whoever is upstream, as it's never gone away for me.
Could you please add some tests, and make it to where if uploading a new image, it is auto-selected?
| } | ||
| } | ||
|
|
||
| export const FilePickerColumn = ({ row }:FilePickerColumnProps) => ( |
There was a problem hiding this comment.
| export const FilePickerColumn = ({ row }:FilePickerColumnProps) => ( | |
| export const FilePickerColumn = ({ row }: FilePickerColumnProps) => ( |
| import PropTypes from 'prop-types'; | ||
| import { | ||
| ActionRow, | ||
| Icon, | ||
| Card, | ||
| Chip, | ||
| Truncate, | ||
| Truncate, Button, |
There was a problem hiding this comment.
| Truncate, Button, | |
| Truncate, | |
| Button, |
| interface FilesPageContextInterface { | ||
| filePickerMode: boolean, | ||
| filePickerOptions?: FilePickerOptions, | ||
|
|
| download: string[]; | ||
| usageMetrics: string[]; | ||
| loading:string; | ||
|
|
| const params = new URLSearchParams(location.search); | ||
| const fileTypes = params.get('fileTypes')?.split(',').map(_type => { | ||
| const type = _type.trim(); | ||
| if (['video', 'audio', 'document', 'image', 'code', 'other'].includes(type)) { |
There was a problem hiding this comment.
Any chance this constant would be valuable elsewhere and could be elevated to the module level (possibly with export?)
export const FILE_TYPES = ['video', 'audio', 'document', 'image', 'code', 'other'] as const|
@Kelketek Surprising running lint:fix didn't fix these. I was sure I ran that before committing, but I just ran it again and it did nothing. I'll fix these issues and rebase. |
…ty, and clean up types
b7e0452 to
651a9ca
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3130 +/- ##
==========================================
+ Coverage 95.70% 95.77% +0.06%
==========================================
Files 1400 1398 -2
Lines 33372 33399 +27
Branches 7819 7589 -230
==========================================
+ Hits 31938 31987 +49
+ Misses 1387 1368 -19
+ Partials 47 44 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…tionality, and clean up types
…er functionality, and clean up types
|
@xitij2000 Thanks for adding the tests here -- I'm not getting auto-select when uploading a file. Could you look at that? |
|
@Kelketek Sorry I forgot to address this earlier. I had started work on this but it was a lot more complex than I expected. So it might be better to get overall approval for this before working on that since it's a bit complex. I have WIP changes stashed for that. |
|
@xitij2000 In that case, would you like to focus on the product proposal and circle back to this, then? |
|
@Kelketek Certainly! |
Description
This creates a new "file_picker" page in the authoring MFE. This page can be used as an embedded file browser for XBlocks to use for selecting assets. When files are selected it posts a message back to the opener with the selected files so they can be used by the block.
Supporting information
This was discussed on the forums and on related issue.
Testing instructions
Currently testing this requires using an XBlock or other code running in the platform that supports this.
Video of above:
Screencast_20260706_191356.webm
Other information
Some of the XBlocks here have been adapted to work with this.
Best Practices Checklist
We're trying to move away from some deprecated patterns in this codebase. Please
check if your PR meets these recommendations before asking for a review:
.ts,.tsx).propTypesanddefaultPropsin any new or modified code.src/testUtils.tsx(specificallyinitializeMocks)apiHooks.tsin this repo for examples.messages.tsfiles have adescriptionfor translators to use.../in import paths. To import from parent folders, use@src, e.g.import { initializeMocks } from '@src/testUtils';instead offrom '../../../../testUtils'