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 pasting attachments on Akkoma / Pleroma instances #735

Merged
merged 30 commits into from
Oct 14, 2024

Conversation

Steffo99
Copy link
Contributor

@Steffo99 Steffo99 commented Sep 4, 2024

Fixes #656.

See commit titles for info on how the bug was fixed!

src/components/compose.jsx Outdated Show resolved Hide resolved
src/utils/store-utils.js Outdated Show resolved Hide resolved
src/components/compose.jsx Outdated Show resolved Hide resolved
src/components/compose.jsx Outdated Show resolved Hide resolved
@cheeaun
Copy link
Owner

cheeaun commented Sep 5, 2024

@Steffo99 I saw the code formatting is a little off. Your code editor might need some prettier plugin to follow the project's prettier settings 🙇

@cheeaun cheeaun added enhancement New feature or request pleroma akkoma labels Sep 5, 2024
@Steffo99
Copy link
Contributor Author

Steffo99 commented Sep 5, 2024

I saw the code formatting is a little off. Your code editor might need some prettier plugin to follow the project's prettier settings 🙇

Oh, I didn't notice the .prettierrc file, and I had my editor configured to use .editorconfig instead; thanks for letting me know!

@Steffo99
Copy link
Contributor Author

Steffo99 commented Sep 5, 2024

What, shouldn't this:

onChange={(e) => {
const files = e.target.files;
if (!files) return;
const mediaFiles = Array.from(files).map((file) => ({
file,
type: file.type,
size: file.size,
url: URL.createObjectURL(file),
id: null, // indicate uploaded state
description: null,
}));
console.log('MEDIA ATTACHMENTS', files, mediaFiles);
// Validate max media attachments
if (
mediaAttachments.length + mediaFiles.length >
maxMediaAttachments
) {
alert(
plural(maxMediaAttachments, {
one: 'You can only attach up to 1 file.',
other: 'You can only attach up to # files.',
}),
);
} else {
setMediaAttachments((attachments) => {
return attachments.concat(mediaFiles);
});
}
// Reset
e.target.value = '';
}}

Share behaviour with this function?:

const handleItems = (e) => {
const { items } = e.clipboardData || e.dataTransfer;
const files = [];
for (let i = 0; i < items.length; i++) {
const item = items[i];
if (item.kind === 'file') {
const file = item.getAsFile();
if (file && supportedMimeTypes.includes(file.type)) {
files.push(file);
}
}
}
if (files.length > 0 && mediaAttachments.length >= maxMediaAttachments) {
alert(
plural(maxMediaAttachments, {
one: 'You can only attach up to 1 file.',
other: 'You can only attach up to # files.',
}),
);
return;
}
console.log({ files });
if (files.length > 0) {
e.preventDefault();
e.stopPropagation();
// Auto-cut-off files to avoid exceeding maxMediaAttachments
const max = maxMediaAttachments - mediaAttachments.length;
const allowedFiles = files.slice(0, max);
if (allowedFiles.length <= 0) {
alert(
plural(maxMediaAttachments, {
one: 'You can only attach up to 1 file.',
other: 'You can only attach up to # files.',
}),
);
return;
}
const mediaFiles = allowedFiles.map((file) => ({
file,
type: file.type,
size: file.size,
url: URL.createObjectURL(file),
id: null,
description: null,
}));
setMediaAttachments([...mediaAttachments, ...mediaFiles]);
}
};

I think that's the reason why the attach button worked but copy-and-pasting not in #656: onChange does not check mime types!

@Steffo99 Steffo99 requested a review from cheeaun September 5, 2024 09:05
@Steffo99
Copy link
Contributor Author

Steffo99 commented Sep 5, 2024

@cheeaun How would you want me to handle this? Should I just leave it alone? In that case, the PR is ready...

src/components/compose.jsx Outdated Show resolved Hide resolved
src/components/compose.jsx Outdated Show resolved Hide resolved
src/components/compose.jsx Outdated Show resolved Hide resolved
src/components/compose.jsx Outdated Show resolved Hide resolved
@Steffo99
Copy link
Contributor Author

Steffo99 commented Sep 7, 2024

Whoops. Committed via web and forgot to enter commit text. A sec!

@Steffo99 Steffo99 force-pushed the feature/paste-attach branch from 8d19420 to 28f87e2 Compare September 7, 2024 20:14
@cheeaun
Copy link
Owner

cheeaun commented Sep 8, 2024

What, shouldn't this:

onChange={(e) => {
const files = e.target.files;
if (!files) return;
const mediaFiles = Array.from(files).map((file) => ({
file,
type: file.type,
size: file.size,
url: URL.createObjectURL(file),
id: null, // indicate uploaded state
description: null,
}));
console.log('MEDIA ATTACHMENTS', files, mediaFiles);
// Validate max media attachments
if (
mediaAttachments.length + mediaFiles.length >
maxMediaAttachments
) {
alert(
plural(maxMediaAttachments, {
one: 'You can only attach up to 1 file.',
other: 'You can only attach up to # files.',
}),
);
} else {
setMediaAttachments((attachments) => {
return attachments.concat(mediaFiles);
});
}
// Reset
e.target.value = '';
}}

Share behaviour with this function?:

const handleItems = (e) => {
const { items } = e.clipboardData || e.dataTransfer;
const files = [];
for (let i = 0; i < items.length; i++) {
const item = items[i];
if (item.kind === 'file') {
const file = item.getAsFile();
if (file && supportedMimeTypes.includes(file.type)) {
files.push(file);
}
}
}
if (files.length > 0 && mediaAttachments.length >= maxMediaAttachments) {
alert(
plural(maxMediaAttachments, {
one: 'You can only attach up to 1 file.',
other: 'You can only attach up to # files.',
}),
);
return;
}
console.log({ files });
if (files.length > 0) {
e.preventDefault();
e.stopPropagation();
// Auto-cut-off files to avoid exceeding maxMediaAttachments
const max = maxMediaAttachments - mediaAttachments.length;
const allowedFiles = files.slice(0, max);
if (allowedFiles.length <= 0) {
alert(
plural(maxMediaAttachments, {
one: 'You can only attach up to 1 file.',
other: 'You can only attach up to # files.',
}),
);
return;
}
const mediaFiles = allowedFiles.map((file) => ({
file,
type: file.type,
size: file.size,
url: URL.createObjectURL(file),
id: null,
description: null,
}));
setMediaAttachments([...mediaAttachments, ...mediaFiles]);
}
};

I think that's the reason why the attach button worked but copy-and-pasting not in #656: onChange does not check mime types!

You mean the filtering logic with supportedMimeTypes? The other one has accept attribute which should already filter the files (I hope).

@Steffo99
Copy link
Contributor Author

Steffo99 commented Sep 8, 2024

The other one has accept attribute which should already filter the files (I hope).

Alas, it doesn't prevent the user from adding files not matching the MIME types listed there, it just filters them by default in the file picker:

Screenshot of me changing file types in the KDE file picker.

I guess that's a bug Something isn't working for a different pull request, though.

@Steffo99 Steffo99 requested a review from cheeaun September 9, 2024 21:06
src/components/compose.jsx Outdated Show resolved Hide resolved
src/components/compose.jsx Outdated Show resolved Hide resolved
@Steffo99 Steffo99 requested a review from cheeaun September 10, 2024 14:57
src/components/compose.jsx Outdated Show resolved Hide resolved
src/components/compose.jsx Outdated Show resolved Hide resolved
@Steffo99 Steffo99 requested a review from cheeaun September 23, 2024 02:20
@cheeaun
Copy link
Owner

cheeaun commented Oct 10, 2024

Possible to resolve the conflicts here? Thanks

@Steffo99
Copy link
Contributor Author

@cheeaun Done!

There seem to be non-prettified files in the main branch; not modifying those to prevent more conflicts.

Steffo99 added a commit to Steffo99/phanpy-junimo that referenced this pull request Oct 13, 2024
@cheeaun cheeaun merged commit 29822cb into cheeaun:main Oct 14, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
akkoma enhancement New feature or request pleroma
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drag-and-drop and paste attachment into compose box
3 participants