-
Notifications
You must be signed in to change notification settings - Fork 199
Conversation
This has to be my name? I don't have any names that are that old. |
You can also test it by making a temporary change, making |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this out. I successfully generated a renew transaction. However, I only saw the transaction ID after renewing. I wasn't sure if that was me signing the TX, or actually submitting it. I clicked 'renew' again, assuming it would then submit the TX. Turns out that I renewed it twice :)
I would recommend changing the UX a bit by:
- Disabling the 'renew' button after successfully submitting a renewal.
- Adding a message that the TX has been submitted, and you should check your identities page in ~1 hour (?) to ensure that the expiration date has been updated
I would also recommend some 'confirm' step before submitting the actual TX. I had no idea what the cost was going to be |
@yknl were you able to incorporate @hstove's suggested changes? They sound helpful to me. Otherwise, this looks functionally like a good addition to me. And I'm wondering if @timstackblock should QA before we go live. |
@hstove is going to take a stab at improving the UX when he gets a chance |
@markmhx I can take a look is this issue pulled into a zenhub board and ready for QA, I dont see it anywhere? |
@timstackblock It sounds from @yknl that @hstove needs to make some UX changes then he'll move this from "In Progress" to "Review / QA" on ZenHub. |
Sure no problem just @ me when you are ready and I will jump right on it. |
@hstove @markmhx any update here? More and more people are going to run into name expiration. |
@diwakergupta I've made a note to discuss this during our weekly kick-off tomorrow. However, iirc we last decided (over the summer) to keep this effort relatively de-prioritized despite the expectation that names will continue to expire since we don't have a reliable way to reach out to users prompting them to renew (e.g. via email). As such, any UX improvements to make renewals easier will have minimal impact. |
@yknl Thanks. I knew how to get to the test page, it was having an id to test with that appeared to require a rebuild. Further, users can only renew with a renewable ID. And the blockstack ID is not renewable because there is no expiration, here is the error I am getting. So, I have to buy an id in order to test this process or get an ID to test with. |
Information related to renewal
Testing Tips from Ken
cc: @yknl please check |
I'm not sure we've settled on this, it seems like a larger product decision. I recommend not emphasizing this for now.
For renewing any ID not in the
No, only the owner can renew the name during the grace period.
For internal testing, you can check that the |
For reviewers: First, you need a "first class domain", as we called it, aka a name like You need BTC to register a name and renew it. Reach out to me if you need some for testing purposes. For testing, I've changed the logic to allow you to renew your name even if it's 2 years away. Previously, we set that to 60 days. We should consider what we want that threshold to actually be when we release it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markmhx Here are my review comments.
app/js/profiles/AllProfilesPage.js
Outdated
@@ -224,8 +228,8 @@ class AllProfilesPage extends Component { | |||
</div> | |||
<div className="row m-t-20"> | |||
<p className="col form-text text-muted"> | |||
Have you recovered and are missing IDs? Just add them | |||
back by using the "Add another ID" for each ID. | |||
Have you recovered and are missing IDs? Just add them back by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does recovered mean here? Recovered from what? Or is it possible to both recover and be missing an ID? Maybe you want to say:
Are you missing IDs after restoring the browser? Use Add another ID for each ID you want to add back.
return 'Renew' | ||
} | ||
if (estimateInProgress) { | ||
return 'Generating estimated cost' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Estimating the purchase price. Price is subject to fluctuations in the price of BTC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the button copy - I think that's too long to put in a button. When the estimate is generated, it says "Renewing your name will cost roughly XXX BTC"
app/js/profiles/RenewNamePage.js
Outdated
{this.state.error ? <div className="alert alert-danger">{this.state.error}</div> : null} | ||
{success && <div className="alert alert-success"> | ||
<p>{success}</p> | ||
<p style={{ marginBottom: 0 }}>Your renewal might take up to two hours to process</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Processing your renewal can take up to two hours.
If you don't like the rephrase, at least take a period for consistency
app/js/profiles/TransferNamePage.js
Outdated
<p> | ||
Transfer ownership is an advanced feature. It requires broadcasting a | ||
transaction on Bitcoin network and costs Bitcoin. | ||
Transfer ownership is an advanced feature. It requires |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Transferring the ownership of a name is an advanced feature. Ownership transfer requires broadcasting a transaction on the Bitcoin network; Transactions cost Bitcoin (BTC).
or
Transferring the ownership of a name is an advanced feature. Ownership transfer requires modifying a name's zone file and broadcasting an update transaction with this change on the Bitcoin network. You must pay for the transaction costs in Bitcoin (BTC).
this.updateAlert('success', 'Broadcasting zone file update transaction...') | ||
this.updateAlert( | ||
'success', | ||
'Broadcasting zone file update transaction...' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Broadcasting name transfer update transaction.
We didn't previously mention the zone file. Are users actually likely to understand what a zone file is? If you use the text I gave you about the zone file up above, then STET this.
app/js/profiles/ZoneFilePage.js
Outdated
Updating your zone file is an advanced feature that can break | ||
your Blockstack name and profile. It requires broadcasting a | ||
transaction on Bitcoin network and costs Bitcoin. | ||
Updating your zone file is an advanced feature that can break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why didn't we mention "breaking" above? Maybe just go for consistency and stick with this text:
Transferring the ownership of a name is an advanced feature. Ownership transfer requires modifying a name's zone file and broadcasting an update transaction with this change on the Bitcoin network. You must pay for the transaction costs in Bitcoin (BTC).
or if we should mention this, this is clearer.
Transferring the ownership of a name is an advanced feature. Ownership transfer requires modifying a name's zone file and broadcasting an update transaction with this change on the Bitcoin network. Further, transfers can fail and leave your name in a state where you can no longer use it. You must pay for the transaction costs in Bitcoin (BTC) regardless of whether the transaction succeeds or fails.
app/js/profiles/ZoneFilePage.js
Outdated
I understand this could break my Blockstack | ||
name and will cost me money. | ||
I understand this could break my Blockstack name and will | ||
cost me money. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I understand I am requesting an ownership transfer of name NAMEHERE. Further, I understand that the transfer can fail leaving my name unavailable. Regardless of whether the owner transfer transaction fails or succeeds, I am required to pay transaction fees in Bitcoin (BTC).
@@ -224,7 +289,7 @@ const registerName = ( | |||
'apps may be unusable until you do.' | |||
if (res.status === 409) { | |||
message = | |||
"Sorry, it looks like we weren't able to process your name registration. Please contact us at support@blockstack.org for help. Some apps may be unusable until you register an ID." | |||
'Sorry, it looks like we weren\'t able to process your name registration. Please contact us at support@blockstack.org for help. Some apps may be unusable until you register an ID.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'Sorry, it looks like the blockchain failed to process your name registration. Please contact us at support@blockstack.org for help. Some apps may be unusable until you register an ID.'
This ^^^ is more decentralized language "we were able to process" makes it look like a central authority.
@@ -251,7 +316,7 @@ const registerName = ( | |||
'apps may be unusable until you do.' | |||
if (e.status === 409) { | |||
message = | |||
"Sorry, it looks like we weren't able to process your name registration. Please contact us at support@blockstack.org for help. Some apps may be unusable until you register an ID." | |||
'Sorry, it looks like we weren\'t able to process your name registration. Please contact us at support@blockstack.org for help. Some apps may be unusable until you register an ID.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'Sorry, it looks like the blockchain failed to process your name registration. Please contact us at support@blockstack.org for help. Some apps may be unusable until you register an ID.'
Thanks @moxiegirl , great feedback! I've updated the copy to what you've suggested. |
This PR adds name expiry information and basic name renewal feature. Full on-chain names that have less than 60 days until expiration will see a renew button beside it. The renewal transaction will be funded by the built-in BTC wallet.
This is based on a previous PR from @friedger to display expiry information in the all profiles screen.
Renew UI in all profiles screen
Subdomains do not need to be renewed
Testing notes