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

Implement add to bookmark page - Closes #2056 #2192

Merged
merged 27 commits into from Jun 28, 2019

Conversation

Projects
3 participants
@massao
Copy link
Contributor

commented Jun 26, 2019

What issue have I solved?

#2056

How have I implemented/fixed it?

  • Created new route /bookmarks/add-bookmark;
  • Created form for adding a new bookmark;
  • Validation of address is token based;
  • Avatar is only generated for valid addresses, and is token based;
  • Should show error if address is invalid or already bookmarked;
  • Label only have validation of length, must be <= 20 character;
  • Was decided with the design team, that we don't need the spinner and checkmark on the inputs on this page;
  • If token is LSK and address belongs to a delegate, username is fetched and label field gets updated and becomes readonly;
  • Switching token should clear the fields;

Obs: After successfully adding a new bookmark, it gets redirected to/dashboard page, due to the /bookmarks page being implemented in another PR: #2187

How has this been tested?

  • Go to /bookmarks/add-bookmark;
  • Fill the inputs with any desired data, it should have the validations on both fields;
    • If address belongs to a delegate, label should be auto filled and readonly;
  • After adding the bookmark, go to /dashboard to check that the bookmark was added, since we don't have the bookmarks page yet;

Review checklist

@massao massao added this to the Sprint 3 milestone Jun 26, 2019

@massao massao self-assigned this Jun 26, 2019

@massao massao added this to Pull Requests in Version 1.19.0 via automation Jun 26, 2019

massao added some commits Jun 26, 2019

@massao massao force-pushed the 2056-implement-add-to-bookmark branch from 90f638c to 5403939 Jun 27, 2019

@massao massao requested a review from slaweet Jun 27, 2019

@massao massao marked this pull request as ready for review Jun 27, 2019

@slaweet
Copy link
Member

left a comment

massao added some commits Jun 28, 2019

massao added some commits Jun 28, 2019

@massao

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2019

Updated to have less space between elements, others spacing are the same as in the design

It was decided with @reyraa and @yasharAyari that since it doesn't rely on a API call, it should just redirect to the bookmarks list.

@massao massao requested a review from slaweet Jun 28, 2019

@slaweet
Copy link
Member

left a comment

Good job, Massao.

@slaweet slaweet requested a review from Efefefef Jun 28, 2019

@Efefefef

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2019

Can you please rebase on development so i can switch tokens?
🐛 You dont clean the readonly input field on change of the address (when there was delegate before)

@Efefefef
Copy link
Contributor

left a comment

I guess you forgot to implement this
image

And margins
image

@massao massao merged commit a34dac4 into development Jun 28, 2019

4 checks passed

Jenkins e2e tests e2e tests passed
Details
Jenkins test deployment Commit was deployed to test
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
coverage/coveralls Coverage decreased (-0.02%) to 94.743%
Details

Version 1.19.0 automation moved this from Pull Requests to Merged Pull Requests Jun 28, 2019

@massao massao deleted the 2056-implement-add-to-bookmark branch Jun 28, 2019

@Efefefef

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2019

Approved, as the last comment is not actual

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.