Skip to content
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

Allow only images in modal image uploader. #7672

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions electron/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,32 @@ app.on('before-quit', () => {
appState.isQuitting = true;
});

// Get the content of a file as a raw buffer of bytes.
// Useful to convert a file path to a File instance.
// Example:
// const result = await ipcMain.invoke('get-file-from-path', 'path/to/file');
// const file = new File([result.buffer], result.name);
ipcMain.handle('get-file-from-path', (event, path) => {
return new Promise((resolve, reject) => {
// Encoding null ensures data results in a Buffer.
fs.readFile(path, { encoding: null }, (err, data) => {
if (err) {
reject(err);
return;
}
// Separate folders considering "\" and "/"
// as separators (cross platform)
const folders = path.split(/[\\/]/);
const fileName = folders[folders.length - 1];
resolve({
name: fileName,
path: path,
buffer: data,
});
});
});
});

ipcMain.on('get-disk-space', async (event) => {
try {
const { data_dir } = await Lbry.settings_get();
Expand Down
29 changes: 22 additions & 7 deletions ui/component/common/file-selector.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// @flow
import * as React from 'react';
import * as remote from '@electron/remote';
import { ipcRenderer } from 'electron';
import Button from 'component/button';
import { FormField } from 'component/common/form';

Expand All @@ -14,6 +15,7 @@ type Props = {
error?: string,
disabled?: boolean,
autoFocus?: boolean,
filters?: Array<{ name: string, extension: string[] }>,
};

class FileSelector extends React.PureComponent<Props> {
Expand Down Expand Up @@ -64,13 +66,26 @@ class FileSelector extends React.PureComponent<Props> {
properties = ['openDirectory'];
}

remote.dialog.showOpenDialog({ properties, defaultPath }).then((result) => {
const path = result && result.filePaths[0];
if (path) {
// $FlowFixMe
this.props.onFileChosen({ path });
}
});
remote.dialog
.showOpenDialog({
properties,
defaultPath,
filters: this.props.filters,
})
.then((result) => {
const path = result && result.filePaths[0];
if (path) {
return ipcRenderer.invoke('get-file-from-path', path);
}
return undefined;
})
.then((result) => {
if (!result) {
return;
}
const file = new File([result.buffer], result.name);
this.props.onFileChosen(file);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned that changing this from { path } to file will break publishing. Did you try?

const publishFormParams: { filePath: string | WebFile, name?: string, optimize?: boolean } = {
expects file.path on electron because the sdk needs the path.

It may be good to send { file: file , path: path } and adjust all the functions assigned to onFileChosen to take that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good @jessopb, nice catch. Perhaps as a small note for the future, we should avoid using these union types. Notice how I'm still respecting the interface (by sending a File type) but still, it manages to break parts of the app...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're probably right. The union types were originally added when the codebase was supporting both web and app.

Maybe the type should be ChosenFile = { file: WebFile, path: string }

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jessopb I have updated the code so the File now contains a path and a mime type. With this, the file upload for posts works fine. Please let me know what you think. Thanks.

});
};

fileInputButton = () => {
Expand Down
13 changes: 12 additions & 1 deletion ui/component/selectAsset/view.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ type Props = {
// passed to the onUpdate function after the
// upload service returns success.
buildImagePreview?: boolean,
// File extension filtering. Files can be filtered
// but the "All Files" options always shows up. To
// avoid that, you can use the filters property.
// For example, to only accept images pass the
// following filter:
// { name: 'Images', extensions: ['jpg', 'png', 'gif'] },
filters?: Array<{ name: string, extension: string[] }>,
type?: string,
};

function filePreview(file) {
Expand All @@ -43,7 +51,8 @@ function filePreview(file) {
}

function SelectAsset(props: Props) {
const { onUpdate, onDone, assetName, currentValue, recommended, title, inline, buildImagePreview } = props;
const { onUpdate, onDone, assetName, currentValue, recommended, title, inline, buildImagePreview, filters, type } =
props;
const [pathSelected, setPathSelected] = React.useState('');
const [fileSelected, setFileSelected] = React.useState<any>(null);
const [uploadStatus, setUploadStatus] = React.useState(SPEECH_READY);
Expand Down Expand Up @@ -121,6 +130,8 @@ function SelectAsset(props: Props) {
/>
) : (
<FileSelector
filters={filters}
type={type}
autoFocus
disabled={uploadStatus === SPEECH_UPLOADING}
label={fileSelectorLabel}
Expand Down
3 changes: 3 additions & 0 deletions ui/modal/modalImageUpload/view.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@ type Props = {

function ModalImageUpload(props: Props) {
const { closeModal, currentValue, title, assetName, helpText, onUpdate } = props;
const filters = React.useMemo(() => [{ name: 'Images', extensions: ['jpg', 'png', 'gif'] }]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this blocks jpeg - possibly some other valid types. svg has been possible and allowed so far.


return (
<Modal isOpen type="card" onAborted={closeModal} contentLabel={title}>
<SelectAsset
filters={filters}
type="openFile"
onUpdate={onUpdate}
currentValue={currentValue}
assetName={assetName}
Expand Down