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.

Sign up for GitHub

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

Jump to bottom

[Storage] Add warning when pinning a cid twice #2027

Merged
merged 10 commits into from
Mar 21, 2022
Merged

[Storage] Add warning when pinning a cid twice #2027

merged 10 commits into from
Mar 21, 2022

Conversation

Tbaut
Copy link
Collaborator

@Tbaut Tbaut commented Mar 17, 2022

closes #1212

@render
Copy link

render bot commented Mar 17, 2022

Your Render PR Server URL is https://chainsafe-components-stage-pr-2027.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c8pmi9j97ejbdv1ivf8g.

@render
Copy link

render bot commented Mar 17, 2022

Your Render PR Server URL is https://storage-ui-stage-pr-2027.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c8pmiaj97ejbdv1ivfd0.

@render
Copy link

render bot commented Mar 17, 2022

Your Render PR Server URL is https://files-ui-stage-pr-2027.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c8pmibj97ejbdv1ivfqg.

@github-actions github-actions bot added the Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes. label Mar 17, 2022
asnaith
asnaith approved these changes Mar 17, 2022
View reviewed changes
Copy link
Member

@asnaith asnaith left a comment

Choose a reason for hiding this comment

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

Good solution for the scenario, warning over prevention makes sense :)

tanmoyAtb
tanmoyAtb requested changes Mar 18, 2022
View reviewed changes
Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

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

So i tried this approach ->
I copied a CID without the last character,
typed in the last character
but I did not get the warning.

While we are at it, On a UI note, I would prefer the CID modal to be wider than what we have.

packages/storage-ui/src/Components/Modules/AddCIDModal.tsx Outdated Show resolved Hide resolved
@Tbaut @tanmoyAtb
Update packages/storage-ui/src/Components/Modules/AddCIDModal.tsx
f39f86a 
Co-authored-by: Tanmoy Basak Anjan <tanmoy3399@gmail.com>
FSM1
FSM1 approved these changes Mar 18, 2022
View reviewed changes
Copy link
Contributor

@FSM1 FSM1 left a comment

Choose a reason for hiding this comment

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

Just a few cosmetic changes, but working great otherwise 👍🏼

@Tbaut
Copy link
Collaborator Author

Tbaut commented Mar 18, 2022

Thanks Tanmoy and Mike. My logic was a little meh for the check. I re-implemented that in a nicer way. The problem you mentioned @tanmoyAtb is now fixed, great catch.

@Tbaut Tbaut requested a review from tanmoyAtb March 18, 2022 14:48
@Tbaut
Copy link
Collaborator Author

Tbaut commented Mar 18, 2022

BTW I agree with the modal style that's a little weird. I'll change that when adding a name together with #2022

tanmoyAtb
tanmoyAtb approved these changes Mar 21, 2022
View reviewed changes
Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

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

Looking 💯

@Tbaut Tbaut merged commit 53b5df5 into dev Mar 21, 2022
@Tbaut Tbaut deleted the mnt/tbaut-warning-pinning-twice-1212 branch March 21, 2022 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FSM1 FSM1 FSM1 approved these changes

@tanmoyAtb tanmoyAtb tanmoyAtb approved these changes

@asnaith asnaith asnaith approved these changes

Assignees
No one assigned
Labels
Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ChainSafe Storage: User can pin the same file multiple times
5 participants
@Tbaut @asnaith @tanmoyAtb @FSM1 @actions-user