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

feat: add by path #708

Merged
merged 10 commits into from
Jul 5, 2018
Merged

feat: add by path #708

merged 10 commits into from
Jul 5, 2018

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Jul 5, 2018

Add files by IPFS path.

image

image

image

image

On this PR, I also created a generic TextInputModal which was used for the modal above and for rename modal.

@hacdias hacdias self-assigned this Jul 5, 2018
@hacdias hacdias added the revamp label Jul 5, 2018
@hacdias hacdias requested review from alanshaw, lidel, olizilla and fsdiogo and removed request for alanshaw, lidel and olizilla July 5, 2018 08:47
@hacdias hacdias mentioned this pull request Jul 5, 2018
18 tasks
@olizilla
Copy link
Member

olizilla commented Jul 5, 2018

Nice! some minor issues when trying it out:

dim on the menu items feels kinda odd, as it makes the hovered thing feel less active rather than more. It could look clearer with bg-animate hover-bg-near-white in place of dim. Your'd need to remove the horizontal padding from the container, and replace the margins with padding on the menu items.

See: http://tachyons.io/docs/themes/skins-pseudo

I just copied and pasted a path from https://archives.ipfs.io/ and it told me it was not valid... turned out there was some rogue whitespace at the front of what I had pasted, but I couldn't see it as the input is narrow and paths are loooooong... can we make the input wider and the font smaller, and trim whitespace on paste pls?

More generally we need to focus on robustness and showing good loading info as the next priority, as the files section is working well, but the UI keeps feeling like it is stuck for me, when I try and preview large files (perhaps we should open large files in a browser tab rather than in line) and large dir listing can take a really long time to load and display.

@hacdias hacdias force-pushed the feat/revamp/add-by-path branch from 7eb9fdc to 830a42c Compare July 5, 2018 11:18
@hacdias
Copy link
Member Author

hacdias commented Jul 5, 2018

dim on the menu items feels kinda odd, as it makes the hovered thing feel less active rather than more.

Done 😄

can we make the input wider and the font smaller

Wouldn't it look odd side by side with other modals even if it's just on the input?

and trim whitespace on paste pls?

Yes, we can.

More generally we need to focus on robustness and showing good loading info as the next priority, as the files section is working well, but the UI keeps feeling like it is stuck for me, when I try and preview large files (perhaps we should open large files in a browser tab rather than in line) and large dir listing can take a really long time to load and display.

Yes, it's the next step. Already mentioned that on #704 (visual feedback).

Copy link
Member

@olizilla olizilla left a comment

Choose a reason for hiding this comment

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

Code looks good. Dropdown should close when you select the by path option

Copy link
Contributor

@fsdiogo fsdiogo left a comment

Choose a reason for hiding this comment

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

Cool 👍

@hacdias hacdias merged commit 7f63a71 into revamp Jul 5, 2018
@hacdias hacdias deleted the feat/revamp/add-by-path branch July 5, 2018 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants