Skip to content
This repository has been archived by the owner on Jun 18, 2019. It is now read-only.

File input refactor and multi-select shares #141

Merged
merged 6 commits into from
Oct 1, 2018
Merged

File input refactor and multi-select shares #141

merged 6 commits into from
Oct 1, 2018

Conversation

liliakai
Copy link
Contributor

@liliakai liliakai commented Aug 9, 2018

Status

Ready for review

Description of Changes

Refactors recover duck to handle multiple files at once and enable multi-select.

Testing

Probably easiest to test this after fix-tests branch is merged to fix the unit tests.

@conorsch conorsch mentioned this pull request Aug 22, 2018
@conorsch
Copy link
Contributor

conorsch commented Sep 7, 2018

Probably easiest to test this after fix-tests branch is merged to fix the unit tests.

#139 is now merged! Mind providing another rebase here? Very much looking forward to having multi-select, that's long been a bear with the current UI. Thanks for tackling, @liliakai !

liliakai and others added 6 commits September 9, 2018 10:58
Simplify FileInput by moving the filename and error state tracking from
FileInput up to FileOrTextInput, because the other use of FileInput
(inside ShareInput) explicitly omits the filename rendering anyway.

This lets us remove the "hack" described in componentWillReceiveProps,
which was intended to propagate the 'clearData' operation to the
FileInput's state. Now that the state is in FileOrTextInput, it can be
updated directly.

This is the first in a series of changes inteded to generalize and
refactor FileInput in an effort to support selecting multiple share
files at once. It doesn't make sense for FileInput to track the state of
a list of files, since it's not designed to render multiple filenames.
This generalizes FileInput so that it always passes an array of one or
more files to it's onChange property, rather than just the attributes of
a single file.

The onChange listeners are updated to handle an array instead of a
single file.

This is another incremental step towards handling multiple shares
selected at the same time.
Now that the RecoverScreen expects an array of file-like objects, the
'Copy from clipboard' button handler needs to massage its input a bit.

This also prevents the ShareStatus from displaying 'undefined' as the
filename for pasted shares, instead showing 'Copied from clipboard'.
@liliakai
Copy link
Contributor Author

liliakai commented Sep 9, 2018

Rebased! 🎉

@conorsch conorsch self-requested a review September 25, 2018 16:13
@conorsch
Copy link
Contributor

@liliakai Please confirm:

Do I have that right? Will prioritize review of this PR over #127 if so.

@liliakai
Copy link
Contributor Author

@liliakai Please confirm:

Do I have that right? Will prioritize review of this PR over #127 if so.

Correct on both counts.

Copy link
Contributor

@conorsch conorsch left a comment

Choose a reason for hiding this comment

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

Works as advertised! Confirmed recovery via multiselect of several shards.

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

Successfully merging this pull request may close these issues.

None yet

2 participants