Feat/pinning settings#1535
Merged
rafaelramalho19 merged 12 commits intoepic/pinning-servicesfrom Jul 15, 2020
Merged
Conversation
rafaelramalho19
commented
Jul 1, 2020
| } | ||
|
|
||
| return ( | ||
| <Modal {...props} className={className} onCancel={onLeave} style={{ maxWidth: '40em' }}> |
Contributor
Author
There was a problem hiding this comment.
This modal is just a placeholder (copied from language modal), will also update it next :)
jessicaschilling
suggested changes
Jul 1, 2020
Contributor
jessicaschilling
left a comment
There was a problem hiding this comment.
This is great! Left one tiny text change inline, but had a few other visual tweaks to get closer to the mockups:
- Font styling in the column headers should be the same as for the column headers in the Files screen
- For "N/A" listings in rows, please make them the same gray as the column headers
- Text baseline in the "Auto Upload" column looks higher than in the other columns
- Icons in the pop-up should be the same color as the icons in the file screen pop-up
- Please add a bit more space between the service's icon and its name to match the mockups (for example, Eternum looks a little tight)
- The margin/padding on the chart itself vis-a-vis its white frame is a little uneven, particularly that it's too little on top and too much on bottom. Could you please match the mockups?
So cool 😊 Thank you!
jessicaschilling
approved these changes
Jul 2, 2020
Contributor
jessicaschilling
left a comment
There was a problem hiding this comment.
@rafaelramalho19 -- I didn't go through the code but visually LGTM based on the screenshots you shared with me in chat. Do you mind posting those in a comment here for archiving purposes and so that @lidel can see, too?
Contributor
Author
|
|
lidel
requested changes
Jul 3, 2020
Member
There was a problem hiding this comment.
Small nits:
- "Local pinning" does not need "three dot menu" – there is nothing to edit, visit, and AFAIK we can't remove it
- As noted in my review of #1539, images should be loaded from IPFS.
(while I was testing me one of third party hostnames was down and image failed to load)
@rafaelramalho19 do we really want to start merging these changes master?
- If we need to make a bugfix release before pinning services are a thing, we have a problem and need to manually hide pinning services UI before release.
- If you are ok with the risk of that additional work, ok with me.
- Translations of new strings will get published to Transifex, but as we iterate on this, those may change, and we add unnecessary work for community members in IPFS translation team.
- If we are extra diligent around proof-reading new labels and ensure they are pretty much solid when merging to
master, it should not be a problem, but wanted to mention the concern here as I know how discouraging it is when you translate something and the same paragraph is changed multiple times withing one or two weeks.
- If we are extra diligent around proof-reading new labels and ensure they are pretty much solid when merging to
If you'd like to play it safe, perhaps create epic/pinning-services branch and create new PRs against that instead? That way pinning services epic work would be isolated from regular dev and when we want to ship pinning services, we would merge everything to master in a single PR.
Contributor
Author
Will fix the small issues you pointed out, thanks 👍 I was thinking about creating a branch called |
Contributor
|
@lidel Thanks for catching that three-dots menu for local services, feeling a bit silly for not seeing that. |
rafaelramalho19
added a commit
that referenced
this pull request
Aug 17, 2020
rafaelramalho19
added a commit
that referenced
this pull request
Sep 29, 2020
rafaelramalho19
added a commit
that referenced
this pull request
Sep 29, 2020
rafaelramalho19
added a commit
that referenced
this pull request
Oct 6, 2020
rafaelramalho19
added a commit
that referenced
this pull request
Oct 6, 2020
rafaelramalho19
added a commit
that referenced
this pull request
Nov 3, 2020
rafaelramalho19
added a commit
that referenced
this pull request
Nov 3, 2020
rafaelramalho19
added a commit
that referenced
this pull request
Nov 10, 2020
rafaelramalho19
added a commit
that referenced
this pull request
Nov 10, 2020
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
rafaelramalho19
added a commit
that referenced
this pull request
Dec 14, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds a mocked-up settings page component for pinning settings.
Left some todo's that I want to fix later, when we:
Demo:
Note: The buttons aren't doing anything on this PR, going to implement it on the next one