extension icon indicating copy to clipboard operation
extension copied to clipboard

update-profile request

Open friedger opened this issue 1 year ago • 8 comments

This PR

  • adds support for update profile requests. These requests are sent from any app and requests that the public profile is updated. The profile is shown to the user before the public profile of the user is updated.
  • fixes #2544

Screenshot from 2022-08-24 01-06-02

cc/ @kyranjamie @fbwoolf @beguene @He1DAr

friedger avatar Aug 23 '22 22:08 friedger

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
stacks-wallet-web ✅ Ready (Inspect) Visit Preview Oct 26, 2022 at 11:12PM (UTC)

vercel[bot] avatar Aug 23 '22 22:08 vercel[bot]

@friedger any way you could provide a test app of sorts that helps us QA this with various inputs?

markmhendrickson avatar Aug 25 '22 10:08 markmhendrickson

Also I see this is in "Draft" – do you prefer we wait on our review until it's out of that state?

markmhendrickson avatar Aug 25 '22 10:08 markmhendrickson

I would like to get early feedback. This is in a state that works.

To get a working app we would need connect support, wouldn't we?

friedger avatar Aug 25 '22 16:08 friedger

Wallet-sdk PR is here: https://github.com/hirosystems/stacks.js/pull/1351

friedger avatar Sep 05 '22 12:09 friedger

@friedger Want to update this PR based on the comments above and let us know when it's ready for another review?

markmhendrickson avatar Sep 07 '22 08:09 markmhendrickson

@friedger it'd be great if we could add integration tests for this flow. Happy to run you through how to write/run these.

kyranjamie avatar Sep 12 '22 08:09 kyranjamie

@kyranjamie thank you. I'll contact you for a date.

@markmhx I try to get this updated as quickly as possible. 😁

friedger avatar Sep 12 '22 11:09 friedger

@friedger I see you've been active on this PR but it's not clear it's ready for re-review. When it is, please reassign @kyranjamie as a reviewer and we'll triage 🙏

markmhendrickson avatar Oct 04 '22 08:10 markmhendrickson

@markmhx In order to get integration tests going we need support by connect: https://github.com/hirosystems/connect/pull/274

friedger avatar Oct 04 '22 13:10 friedger

image

Profile updating via test app works.

Next:

  • fix missing properties in connect: https://github.com/hirosystems/connect/pull/277
  • add proper integration tests with the help of @kyranjamie
  • define supported profile properties with @markmhx (?)

friedger avatar Oct 05 '22 20:10 friedger

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: friedger
:x: fbwoolf
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Oct 19 '22 21:10 CLAassistant

I have updated the screenshot in the description with the current design.

This PR just needs a new release of connect (https://github.com/hirosystems/connect/pull/279) using stacks.js from https://github.com/hirosystems/stacks.js/pull/1381

friedger avatar Oct 19 '22 21:10 friedger

I'm glad to see this coming together!

Can we improve the data formatting a bit, per the same guidance provided here? https://github.com/hirosystems/stacks-wallet-web/pull/2405#issuecomment-1123780529

Also is the red circle supposed to be the profile photo? I wonder if we should display the image URL raw like other attributes for now, and save displaying it for a future enhancement where we look at customizing this screen in general for standardized attributes?

Update: I see a few other UI nits, so I'll provide a design mockup soon to cover them all as guidance

markmhendrickson avatar Oct 20 '22 11:10 markmhendrickson

Profile image is shown in addition to the plain data. For the image, it is image[0].contentUrl

friedger avatar Oct 20 '22 12:10 friedger

@friedger Here's a mockup with some general formatting and copy improvements. What do you think?

image

Figma version here

markmhendrickson avatar Oct 21 '22 15:10 markmhendrickson

@markmhx Looks good. How are array items separated from each other? Like two images.

friedger avatar Oct 21 '22 18:10 friedger

I've updated the above mockup to include guidance for arrays of objects:

image

This seems to correspond to how arrays in structured messages are formatted:

Screen Shot 2022-10-24 at 14 03 25

markmhendrickson avatar Oct 24 '22 12:10 markmhendrickson

sameAs is also an array. Between two arrays, there is only a ,, not ],[

friedger avatar Oct 24 '22 16:10 friedger

Oops, fixed?

image

markmhendrickson avatar Oct 24 '22 16:10 markmhendrickson

I have replaced the screenshot in the PR description with the current state.

Known issues:

  • The screenshot shows "requested by unknown app" due to an older version of connect. This is fixed in https://github.com/hirosystems/connect/pull/279/commits/9d3c7401fa9166af2cf364d68565bf0d80cb0ff4.
  • connect-react does not yet support updating profiles.

This PR is now ready for review!

friedger avatar Oct 24 '22 19:10 friedger

Nice work! We'll review.

UI-wise, mind just bumping the image down so it top-aligns with the top of the header?

image image

markmhendrickson avatar Nov 03 '22 16:11 markmhendrickson

@friedger is attempting to deploy a commit to the Blockstack Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Nov 03 '22 22:11 vercel[bot]

@markmhx Voila! image

friedger avatar Nov 03 '22 22:11 friedger

@friedger Thanks! Mind resolving the lint error?

markmhendrickson avatar Nov 07 '22 15:11 markmhendrickson

@markmhx commits have been fixed. The failing integration test is part of dev branch I think

friedger avatar Nov 07 '22 18:11 friedger

One of us might have to cherry pick these commits to fix the failing test. Nice work @friedger, thanks for your help on this. I just had some minor comments about utilizing UI library elements in some places, but I'm sure one of us can address them if needed.

fbwoolf avatar Nov 07 '22 19:11 fbwoolf

@kyranjamie Mind helping to generate a test build here for QA? Is it not getting generated due to the Snyk error?

markmhendrickson avatar Nov 09 '22 13:11 markmhendrickson

Waiting for the approval of the update-profile request functionality as it has become essential for our Dapp CrossCheck xck.app next release. We have already integrated it into the dapp, and we are still conducting tests, which we hope to share on Monday. The actual version stores all the profile data in the GAIA, this new functionality gives more flexibility to multi-app compatibility. Now happily feeling the pressure for around 10K new users for xck.app, so eager to advance in this topic, before distribution. Thanks!

paradigma-cl avatar Nov 12 '22 16:11 paradigma-cl

Test build getting generated here: https://github.com/hirosystems/stacks-wallet-web/pull/2829

markmhendrickson avatar Nov 14 '22 15:11 markmhendrickson