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: APP-183 attach signature to data post + APP-240 #2423

Merged
merged 13 commits into from
Aug 29, 2024

Conversation

blushi
Copy link
Member

@blushi blushi commented Aug 5, 2024

Description

https://regennetwork.atlassian.net/browse/APP-183?atlOrigin=eyJpIjoiYTczMWFmNjQ1NjhkNGU2Y2FiYTk2NzYzM2E5YWJkMjIiLCJwIjoiaiJ9

https://regennetwork.atlassian.net/browse/APP-240?atlOrigin=eyJpIjoiZmNkYmEwMTIwNjM0NGM5MjlhZjhmNGUxN2JjZmYwMzMiLCJwIjoiaiJ9


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • provided a link to the relevant issue or specification
  • provided instructions on how to test
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

How to test

From https://deploy-preview-2423--regen-marketplace.netlify.app/

  1. Log in with a web2 account and create a post, you should see anchoring hash in the success modal
  2. Log in with a web3 account and create a post. After entering post data, you should see a modal to sign the post or skip this step. Test both options. If signing a post successfully, you'll see the attesting hash in the success modal too.

The button at the bottom of the success modal now goes to the newly created post page.

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items
.

I have...

  • confirmed all author checklist items have been addressed
  • reviewed code correctness and readability
  • verified React components follow DRY principles
  • reviewed documentation is accurate
  • reviewed tests
  • manually tested (if applicable)

Copy link

netlify bot commented Aug 5, 2024

Deploy Preview for regen-website ready!

Name Link
🔨 Latest commit 11f1037
🔍 Latest deploy log https://app.netlify.com/sites/regen-website/deploys/66d075df8008cc000839710d
😎 Deploy Preview https://deploy-preview-2423--regen-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@blushi blushi changed the title feat: APP-183 sign post feat: APP-183 attach signature to data post Aug 5, 2024
@blushi blushi changed the title feat: APP-183 attach signature to data post feat: APP-183 attach signature to data post + APP-240 Aug 6, 2024
@blushi blushi requested review from clevinson and a team August 6, 2024 08:37
@blushi
Copy link
Member Author

blushi commented Aug 6, 2024

@erikalogie @clevinson see testing instructions

@erikalogie
Copy link
Collaborator

@blushi when creating the post from the profile/projects page with a web 2.0 account, I never got the success modal though the post was successfully created

For the web 3.0 accounts, the signing modal looks like this and then I also don't get the success modal:
Screenshot 2024-08-06 at 9 23 32 AM

Screenshot 2024-08-06 at 9 23 23 AM

@blushi
Copy link
Member Author

blushi commented Aug 7, 2024

@erikalogie could you test this again?

@erikalogie
Copy link
Collaborator

erikalogie commented Aug 7, 2024

It is working well for me now! Just double-check the font size for the hashes in the confirmation popup after a user has signed, does not match the comps or the other confirmation popups.

@blushi
Copy link
Member Author

blushi commented Aug 8, 2024

It is working well for me now! Just double-check the font size for the hashes in the confirmation popup after a user has signed, does not match the comps or the other confirmation popups.

@erikalogie could you have another look?

@erikalogie
Copy link
Collaborator

Looks great, one last tiny detail, these don't seem to be perfectly left-aligned on mobile:
Regen Marketplace : Buy, trade and retire carbon credits | Regen Marketplace : Buy, trade and retire carbon credits 2024-08-08 08-56-34

@clevinson
Copy link
Member

This looks great!

Screenshot 2024-08-08 at 3 09 00 PM Screenshot 2024-08-08 at 8 57 42 PM

I just have a few copy suggestions.

When no signature is provided, there's no title in the modal that pops up. Additionally I think the "Attest" title should be a bit more clear.

Can we instead have the title always say "Create Data Post", similar to how when creating a project it says "Create Project"

Then, in the section below I think we should change the following:

  1. "Hash" section header should be "Blockchain Record"
  2. "Anchor" should be "Create Post"
  3. "Attest" should be "Signature"

I think those will be more clear to non web3 users. @erikalogie @blushi let me know what y'all think? In general I think we can leave "Anchor" and "Attest" out of the marketplace UIs, and have them only appears in API documentation and Mintscan.

@erikalogie
Copy link
Collaborator

This looks great!

Screenshot 2024-08-08 at 3 09 00 PM Screenshot 2024-08-08 at 8 57 42 PM
I just have a few copy suggestions.

When no signature is provided, there's no title in the modal that pops up. Additionally I think the "Attest" title should be a bit more clear.

Can we instead have the title always say "Create Data Post", similar to how when creating a project it says "Create Project"

Then, in the section below I think we should change the following:

  1. "Hash" section header should be "Blockchain Record"
  2. "Anchor" should be "Create Post"
  3. "Attest" should be "Signature"

I think those will be more clear to non web3 users. @erikalogie @blushi let me know what y'all think? In general I think we can leave "Anchor" and "Attest" out of the marketplace UIs, and have them only appears in API documentation and Mintscan.

All sounds good to me. If we are going to change "hash" here then we should probably change it on all the other confirmation popups too for consistency.

@blushi
Copy link
Member Author

blushi commented Aug 26, 2024

@erikalogie @clevinson ready for another review, I've applied the copy suggestions.

@blushi
Copy link
Member Author

blushi commented Aug 27, 2024

@clevinson @erikalogie we also now have this "NOTE: As posts are anchored to the blockchain, they are not editable once published" at the bottom of the post creation form. Should we rephrase it to get rid of "anchored"?

@erikalogie
Copy link
Collaborator

erikalogie commented Aug 27, 2024

@clevinson @erikalogie we also now have this "NOTE: As posts are anchored to the blockchain, they are not editable once published" at the bottom of the post creation form. Should we rephrase it to get rid of "anchored"?

Yes, I think @clevinson will likely have a good suggestion here.

@erikalogie
Copy link
Collaborator

@erikalogie @clevinson ready for another review, I've applied the copy suggestions.

The copy looks like nonsense again:
Screenshot 2024-08-27 at 10 27 10 AM

@blushi
Copy link
Member Author

blushi commented Aug 28, 2024

@erikalogie @clevinson ready for another review, I've applied the copy suggestions.

The copy looks like nonsense again: Screenshot 2024-08-27 at 10 27 10 AM

Sorry this is when I forget to generate the .po files for the translation now that we have it set up in the app. I'll fix that.

@blushi
Copy link
Member Author

blushi commented Aug 28, 2024

Sorry this is when I forget to generate the .po files for the translation now that we have it set up in the app. I'll fix that.

@erikalogie this should be fixed now

@erikalogie
Copy link
Collaborator

LGTM

@blushi blushi merged commit 5373c43 into dev Aug 29, 2024
14 checks passed
@blushi blushi deleted the feat-APP-183-sign-post branch August 29, 2024 13:29
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.

4 participants