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

NEW Move the replace file into the more options action set #848

Merged
merged 2 commits into from
Oct 29, 2018

Conversation

sachajudd
Copy link
Contributor

Resolves #757

@@ -70,6 +70,15 @@ class AssetDropzone extends Component {
}
}

componentDidUpdate() {
// Reattach name to hiddenFileInput as dropzone recreates this element after each upload
const name = this.props.name;
Copy link
Contributor

Choose a reason for hiding this comment

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

We'd prefer this to be destructured: const { name } = this.props;

const hiddenFileInput = document.querySelector('.dz-input-PreviewImage');

// Trigger a click on Dropzone's hidden file input in order to upload an image
hiddenFileInput.click();
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably worth checking that hiddenFileInput is resolved before calling click on it

*/
protected function getReplaceFileAction($record)
{
$action = FormAction::create('replacefile', 'Replace file')
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please make the field title translatable?

@robbieaverill
Copy link
Contributor

Also looks the tests need some love =)

@unclecheese unclecheese changed the base branch from 1 to 1.3 October 16, 2018 01:19
@unclecheese
Copy link

Would be good to get this into the 4.3 release. Looks it's just some minor revisions from @sachajudd ? Have optimistically retargeted to 1.3.

@sachajudd sachajudd force-pushed the pulls/1/replace-file branch from 84af47b to 7a06d1d Compare October 16, 2018 21:12
@sachajudd
Copy link
Contributor Author

Sounds good @unclecheese, I'll get on to looking at the tests 🙂

@sachajudd sachajudd force-pushed the pulls/1/replace-file branch 2 times, most recently from 01a63f4 to 6423656 Compare October 18, 2018 22:42
@sachajudd sachajudd requested a review from unclecheese October 18, 2018 23:23
@sachajudd sachajudd force-pushed the pulls/1/replace-file branch from 6423656 to f2b9ac0 Compare October 23, 2018 04:10
@bergice bergice self-assigned this Oct 25, 2018
@bergice bergice removed their assignment Oct 26, 2018
@unclecheese
Copy link

Looking good. I'm going to add a behat test for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants