- Notifications
You must be signed in to change notification settings - Fork1.1k
Restore Missing fileInputId Property and Add acceptedFileTypes Property#1089
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
afcapel left a comment
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.
There are a couple of styling issues, but the changes make sense. If you can fix them I'll merge.
.DS_Store Outdated
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.
Can you remove this file, please?
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.
Removed. I've also updated.gitignore to help avoid this. I hope this is OK. I see it's included there inKamal.
src/trix/config/input.js Outdated
| constinput={ | ||
| level2Enabled:true, | ||
| fileInputId:`trix-file-input-${Date.now().toString(16)}`, | ||
| acceptedFileTypes:'*', |
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.
We prefer double quotes for strings. The linter should flag this.
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.
Fixed.
chadrschroeder commentedOct 2, 2023
Thanks for the quick review before. Let me know if there's anything else you want me to update here. It looks like the most recent tests failed but in a way that is unrelated to my changes. |
ghiculescu commentedJul 25, 2024
Is this okay to merge@afcapel ? |
afcapel commentedJul 25, 2024
@ghiculescu I afraid I don’t maintain Trix anymore, so that ‘s up to the new maintainers. |
Fixes#1088 by restoring the missing
fileInputIdproperty.Also adds an
acceptedFileTypesproperty, similar to what was proposed in#789. That PR was opened over 3 years ago back when this code was still using CoffeeScript. I've implemented a similar change here in JavaScript, but I set this up as a simple string attribute value instead of an array. Let me know if you'd rather not have this option, but this is why I was trying to override thepickFilesfunction in the first place.