feat: add delete prompt to file browser #689
Conversation
| @@ -35,17 +42,17 @@ class FilesPage extends React.Component { | |||
| doUpdateHash(`/files${link}`) | |||
| } | |||
|
|
|||
| onInspect = (hash) => { | |||
| onInspect = ([file]) => { | |||
There was a problem hiding this comment.
Why add the array here?
PS: I'm dropping in without much background.
There was a problem hiding this comment.
We were only sending the required info for each hook: the delete only needed paths, the inspect only needed the hash, etc.
But since now we also need the type of the file, we would have to send two different things for delete hook.
I then decided just to send everything to every hook and the hook uses the data it wants. You can see the changes on this commit.
src/files/FilesPage.js
Outdated
| let filesNumber = 0 | ||
| let foldersNumber = 0 | ||
|
|
||
| files.forEach(file => { |
There was a problem hiding this comment.
Don't you prefer one liners when it's a simple conditional?
files.forEach(file => file.type === 'directory' ? foldersNumber++ : filesNumber++)There was a problem hiding this comment.
For me it's indifferent, I don't think one way is better than the other. I can change it though.
hacdias
force-pushed
the
feat/revamp/delete-prompt-applied
branch
from
6826b6b
to
0a0c2cd
Compare
|
@olizilla let's merge this 😄 |
|
👀 |
|
There's an issue with the focus outline. The buttons are inheriting the focus state from the parent which looks weird. focus should generally only apply to a single element and isn't inherited. My bad, the |
|
@olizilla just fixed 😄
|
The commits on this PR are self contained and should be merged without squashing.