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 pinning to files page #1678

Merged
merged 15 commits into from
Nov 3, 2020

Conversation

rafaelramalho19
Copy link
Contributor

@rafaelramalho19 rafaelramalho19 commented Oct 26, 2020

Description:
➕ Updated/added pin icons
➕ Added new translations for pinning context menu + modal (please review them @jessicaschilling ?)
🐛 Fixed a bug where the file list with a lot of files (enough to have a scrollbar) wouldn't show the overlay correctly when dragging files to upload
💻 Refactored whole files page component to make it modern and faster ❤️

@rafaelramalho19 rafaelramalho19 requested review from jessicaschilling and lidel and removed request for jessicaschilling October 26, 2020 19:20
Copy link
Contributor

@jessicaschilling jessicaschilling left a comment

Choose a reason for hiding this comment

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

Thank you! 😊 Added some inline comments for some small visual tweaks, but a few other notes, please:

  1. Please add column header of Pin status per mockups; ideally this would be sortable but OK to omit for initial release if that's too painful.
    image

  2. Per mockups, please add Set pinning option in multi-select bottom bar:
    image

  3. I'm having trouble viewing the third-party pinning service icons, for what that's worth:
    image

  4. Are the checkboxes meant to work yet? (They don't for me.)

  5. I think the icons in all the modals died 😢 Example: there's no trash icon in the Delete modal any more ...
    image

src/files/file/File.js Outdated Show resolved Hide resolved
src/files/file/File.js Outdated Show resolved Hide resolved
src/files/file/File.js Outdated Show resolved Hide resolved
src/files/file/File.js Outdated Show resolved Hide resolved
src/files/modals/pinning-modal/PinningModal.js Outdated Show resolved Hide resolved
src/files/modals/pinning-modal/PinningModal.js Outdated Show resolved Hide resolved
src/files/modals/pinning-modal/PinningModal.js Outdated Show resolved Hide resolved
src/files/modals/pinning-modal/PinningModal.js Outdated Show resolved Hide resolved
src/files/modals/pinning-modal/PinningModal.js Outdated Show resolved Hide resolved
rafaelramalho19 and others added 11 commits October 27, 2020 13:53
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
@rafaelramalho19
Copy link
Contributor Author

I fixed all the icons in all modals, I broke it by mistake. Thanks for noticing it 😄

Copy link
Contributor

@jessicaschilling jessicaschilling left a comment

Choose a reason for hiding this comment

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

This seems great (issues will probably crop up during implementation, but this seems like good "furniture" for now) 😊

One thing: On the Files page, no columns seem sortable now? Even if the Pin Status column isn't sortable, Name and Size should still be.

@rafaelramalho19
Copy link
Contributor Author

This seems great (issues will probably crop up during implementation, but this seems like good "furniture" for now) 😊

One thing: On the Files page, no columns seem sortable now? Even if the Pin Status column isn't sortable, Name and Size should still be.

That's actually broken in master after the actions refactor (I believe), I'll fix it here too 😄

Copy link
Contributor

@jessicaschilling jessicaschilling left a comment

Choose a reason for hiding this comment

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

much good, very sort
thanks! 😊

@lidel lidel changed the title Feat/add pinning to files page feat: add pinning to files page Nov 2, 2020
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

lgtm!

nit: I've found small UI bug in Firefox when you quicky scrolling a big directory.
Not sure if its Linux-specific, but try Browsing QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm and then scrolling it quickly in Firefox. I see this flashy behavior:

firefox-scroll-artifacts

@rafaelramalho19 / @jessicaschilling are you able to reproduce (on Firefox)?

@rafaelramalho19
Copy link
Contributor Author

@lidel Couldn't reproduce, is that something that happens to you in master too?

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

@rafaelramalho19 yup, its also in master, so not in scope of this PR.

LGTM!

@rafaelramalho19 rafaelramalho19 merged commit bc60e46 into epic/pinning-services Nov 3, 2020
@rafaelramalho19 rafaelramalho19 deleted the feat/add-pinning-to-files-page branch November 3, 2020 15:02
rafaelramalho19 added a commit that referenced this pull request Nov 3, 2020
* chore: refactor files page

* feat: add pinning services mock to files page

* chore: update remotePin dimensions

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update local pin icon dimension

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: change pin icon fill color

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: change remote pin icon fill color

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal text size

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal text size

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: pinning modal secondary text changes

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal image size

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal pin icon

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: add pin status column to fileslist

* chore: fix modals icons

* chore: fix linting error

* chore: fix files sorting

Co-authored-by: Jessica Schilling <jessica@protocol.ai>
rafaelramalho19 added a commit that referenced this pull request Nov 10, 2020
* chore: refactor files page

* feat: add pinning services mock to files page

* chore: update remotePin dimensions

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update local pin icon dimension

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: change pin icon fill color

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: change remote pin icon fill color

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal text size

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal text size

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: pinning modal secondary text changes

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal image size

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal pin icon

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: add pin status column to fileslist

* chore: fix modals icons

* chore: fix linting error

* chore: fix files sorting

Co-authored-by: Jessica Schilling <jessica@protocol.ai>
rafaelramalho19 added a commit that referenced this pull request Dec 14, 2020
* Feat/pinning settings (#1535)

* Feat/pinning settings custom modal (#1546)

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* Feat/redesign files bar (#1513)

* Feat/pinning settings (#1535) (#1557)

* feat: add pinning services service modal

* chore: update translation for service modal

* chore: fix tslint in directory selector

* chore: fix tscheck issues in upper directory selector

* chore: fix tslint in selectors

* feat: add pinning to files page (#1678)

* chore: refactor files page

* feat: add pinning services mock to files page

* chore: update remotePin dimensions

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update local pin icon dimension

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: change pin icon fill color

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: change remote pin icon fill color

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal text size

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal text size

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: pinning modal secondary text changes

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal image size

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal pin icon

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: add pin status column to fileslist

* chore: fix modals icons

* chore: fix linting error

* chore: fix files sorting

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* feat: prepare pinning services for stage 1

* chore: remove remote pins from settings page for stage 1

* chore: update pinning manager padding

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update modal horizontal padding

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: make settings page pinning table responsive

* chore: update settings page description in pinning table

* chore: update header style

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update settings page in smaller viewports

* chore: remove outline on pinning tables focus

* chore: update translation

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update translation

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: add titles to bar options

* chore: add titles to bar options

* Update public/locales/en/files.json

* feat: add pins size to the settings page

* feat: add number of pins to settings page

* chore: feat tslint

* chore: fix error in button

* test(e2e): more reliable api test suite

This changes the way we enter API address/config from programmatic
to full simulation of user input and adds tiny slow down between each
key stroke.

This should solve the problem of newly added UI feedback not engaging,
and make CI both more reliable and green again.

While at it, made it CI-agnostic, in preparation for move to
GithubActions

Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Co-authored-by: Marcin Rataj <lidel@lidel.org>
rafaelramalho19 added a commit that referenced this pull request Dec 14, 2020
* chore: refactor files page

* feat: add pinning services mock to files page

* chore: update remotePin dimensions

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update local pin icon dimension

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: change pin icon fill color

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: change remote pin icon fill color

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal text size

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal text size

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: pinning modal secondary text changes

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal image size

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: update pinning modal pin icon

Co-authored-by: Jessica Schilling <jessica@protocol.ai>

* chore: add pin status column to fileslist

* chore: fix modals icons

* chore: fix linting error

* chore: fix files sorting

Co-authored-by: Jessica Schilling <jessica@protocol.ai>
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